From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bounce+27952+110931+7686176+12367111@groups.io>
Received: from mail02.groups.io (mail02.groups.io [66.175.222.108])
	by spool.mail.gandi.net (Postfix) with ESMTPS id AB7FB7803CC
	for <rebecca@openfw.io>; Wed,  8 Nov 2023 22:21:32 +0000 (UTC)
DKIM-Signature: a=rsa-sha256; bh=PKAnEcXDFUhOdYwRY2iR8lbJGYUAxPxVy+BI2CJK0W8=;
 c=relaxed/simple; d=groups.io;
 h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding;
 s=20140610; t=1699482091; v=1;
 b=huhok7unP11BL7wrlRSMtNA7vwjdbGsoLYaDEfgRlWGvJWismDnEwnFFW6HMJl5oIJlEcWbE
 M6XzsanI59hGFFkTMTFWPZ3PxCO9JPRCMKI5/+PMrpenEMnT6sjbFrpSczP5MYZmhxxsMgb07OS
 alRiigpDGelF+qKE2HTYuNPk=
X-Received: by 127.0.0.2 with SMTP id 3RDIYY7687511xP2GD5chUqu; Wed, 08 Nov 2023 14:21:31 -0800
X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124])
 by mx.groups.io with SMTP id smtpd.web11.106063.1699482090669905526
 for <devel@edk2.groups.io>;
 Wed, 08 Nov 2023 14:21:30 -0800
X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com
 [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
 us-mta-517-ShmhHkZLN8SKVi4uIr2R7A-1; Wed, 08 Nov 2023 17:21:25 -0500
X-MC-Unique: ShmhHkZLN8SKVi4uIr2R7A-1
X-Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
	(No client certificate requested)
	by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5F766A28802;
	Wed,  8 Nov 2023 22:21:25 +0000 (UTC)
X-Received: from [10.39.192.41] (unknown [10.39.192.41])
	by smtp.corp.redhat.com (Postfix) with ESMTPS id 87738492BE8;
	Wed,  8 Nov 2023 22:21:23 +0000 (UTC)
Message-ID: <95147d56-ee9b-ebf7-6e69-7e41402f4cf0@redhat.com>
Date: Wed, 8 Nov 2023 23:21:21 +0100
MIME-Version: 1.0
Subject: Re: [edk2-devel] [PATCH v2 20/30] OvmfPkg/LoongArchVirt: Add serial port library
To: devel@edk2.groups.io, lichao@loongson.cn, kraxel@redhat.com
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
 Jiewen Yao <jiewen.yao@intel.com>, Jordan Justen
 <jordan.l.justen@intel.com>, Xianglai Li <lixianglai@loongson.cn>
References: <20231106032521.2251143-1-lichao@loongson.cn>
 <20231106032945.2285855-1-lichao@loongson.cn>
 <lzbp5prfycfvb37bsbwszhnjmoxh73j342mbfehhygyffdxylc@i3nuhvuhgvke>
 <0d008dd7-2622-4d8b-9ad6-d0381d797e47@loongson.cn>
From: "Laszlo Ersek" <lersek@redhat.com>
In-Reply-To: <0d008dd7-2622-4d8b-9ad6-d0381d797e47@loongson.cn>
X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Precedence: Bulk
List-Subscribe: <mailto:devel+subscribe@edk2.groups.io>
List-Help: <mailto:devel+help@edk2.groups.io>
Sender: devel@edk2.groups.io
List-Id: <devel.edk2.groups.io>
Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io
Reply-To: devel@edk2.groups.io,lersek@redhat.com
List-Unsubscribe-Post: List-Unsubscribe=One-Click
List-Unsubscribe: <https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/plugh>
X-Gm-Message-State: yGx9Bq5aFBVFmMwpefLzIR0Ox7686176AA=
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
X-GND-Status: LEGIT
Authentication-Results: spool.mail.gandi.net;
	dkim=pass header.d=groups.io header.s=20140610 header.b=huhok7un;
	dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none);
	spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io

On 11/7/23 11:12, Chao Li wrote:
> Hi Gerd,
>=20
> These two libraries is not only copy code, the way of obtain the serial
> base address is different from ARM, and the early serial port output
> also different from ARM, so these two libraries are LoongArch specific.

I think we're going to have to go through the design with "baby steps".

The commit message of the patch is empty.

Please don't expect reviewers to "reverse engineer" the design from the
code. That's hugely taxing. It's hard enough to review code when we
precisey know in advance, from the commit message, what the code will
try to achieve.

You are saying that the way to obtain serial base address is different
from ARM, yet the GetSerialConsolePortAddress() function looks very
similar to the function that I *replaced* in recent commits eb83b5330961
("ArmVirtPkg: introduce FdtSerialPortAddressLib", 2023-10-26) and
f078a6fdd4d7 ("ArmVirtPkg/Fdt16550SerialPortHookLib: rebase to
FdtSerialPortAddressLib", 2023-10-26).

So there are two issues with your GetSerialConsolePortAddress() function
immediately, I would say:

- you say that it's different from ARM, but it seems to parse the DTB
identically (or mostly identically -- I'm speaking from memory),

- I factored out the subject DTB parsing logic in the above-noted
commits recently, so even if your GetSerialConsolePortAddress() function
is *supposed* to parse the DTB identically to ARM, then the way to
employ that logic is different; it should be reused, not duplicated.

Now that you have a running prototype (basically evidence that the port
to this new architecture can be done), can we go through the design of
each component (library, driver etc) one by one? Dumping the whole thing
on reviewers all at once, with little documentation, is not helpful. We
basically need to start with the specification of each of your modules.
See where the existing components in edk2 are not good enough or not
flexible enough, etc.

Laszlo



-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110931): https://edk2.groups.io/g/devel/message/110931
Mute This Topic: https://groups.io/mt/102413885/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562=
12/xyzzy [rebecca@openfw.io]
-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-