Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 4 Nov 2021 04:30:14 -0500
From:   Patrick Williams <patrick@stwcx.xyz>
To:     Howard Chiu =?utf-8?B?KOmCseWGoOedvyk=?= 
        <Howard.Chiu@quantatw.com>
Cc:     Howard Chiu <howard10703049@gmail.com>,
        "arnd@arndb.de" <arnd@arndb.de>, "olof@lixom.net" <olof@lixom.net>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "joel@jms.id.au" <joel@jms.id.au>,
        "andrew@aj.id.au" <andrew@aj.id.au>,
        "soc@kernel.org" <soc@kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>
Subject: Re: [PATCH v1] ARM: dts: aspeed: Adding Facebook Bletchley BMC
Message-ID: <YYOoJgXyOdgNmI6B@heinlein>
References: <20211103071417.388388-1-howard.chiu@quantatw.com>
 <YYKatBCCroiYxLew@heinlein>
 <HKAPR04MB40039608E14195D859DE7EC5968D9@HKAPR04MB4003.apcprd04.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="pNxo54Czgw/SMy95"
Content-Disposition: inline
In-Reply-To: <HKAPR04MB40039608E14195D859DE7EC5968D9@HKAPR04MB4003.apcprd04.prod.outlook.com>
Precedence: bulk


--pNxo54Czgw/SMy95
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 04, 2021 at 03:30:08AM +0000, Howard Chiu (=E9=82=B1=E5=86=A0=
=E7=9D=BF) wrote:

> > Is this board using 64MB or 128MB modules?  Many of the newer systems
> > have been
> > starting to use 128MB.  I just want to confirm this is correct.
> 1Gb SPI flash, MX66L1G45GMI-08G

1Gb =3D 1024Mb / 8 =3D 128MB, right?  Shouldn't we use the 128MB layout?

> > > +	sled0_ioexp: pca9539@76 {
> > > +		compatible =3D "nxp,pca9539";
> > > +		reg =3D <0x76>;
> > > +		#address-cells =3D <1>;
> > > +		#size-cells =3D <0>;
> > > +		gpio-controller;
> > > +		#gpio-cells =3D <2>;
> > > +
> > > +		gpio-line-names =3D
> > > +
> > 	"","SLED0_BMC_CCG5_INT","SLED0_INA230_ALERT","SLED0_P12V_STBY_
> > ALERT",
> > > +
> > 	"SLED0_SSD_ALERT","SLED0_MS_DETECT","SLED0_MD_REF_PWM","",
> > > +
> > 	"SLED0_MD_STBY_RESET","SLED0_MD_IOEXP_EN_FAULT","SLED0_MD_D
> > IR","SLED0_MD_DECAY",
> > > +
> > 	"SLED0_MD_MODE1","SLED0_MD_MODE2","SLED0_MD_MODE3","SLED
> > 0_AC_PWR_EN";
> >=20
> > In general, in OpenBMC, we have a preference for the GPIOs to not be
> > schematic
> > names but to be named based on their [software-oriented] function.  Ple=
ase
> > take
> > a look at:
> >=20
> >=20
> > https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-na
> > ming.md
> >=20
> > Any function you see that isn't documented there we should try to get
> > documented
> > before fixing the GPIO name to match it.
> >=20
> I intend to delete them for now if I have to document them first, because=
=20
> most of them are platform-specific GPIO, not for general purpose and also=
 not
> suitable to current OpenBMC.
> For example, OpenBMC believes there is only one GPIO to be used to power =
on=20
> the chassis, but Bletchley has six.
> I define gpio-line-names for gpioset/geioget/phosphor-multi-gpio-monitor
> usage, and they can be replaced with gpiochip number and offset instead.
> The disadvantage is that they won't be human-friendly when TEs develop th=
eir tool to test these GPIOs.
> > > +		gpio-line-names =3D
> > > +		"SLED0_EMBER_LED","SLED0_BLUE_LED","SLED0_RST_IOEXP","",

Deleting them entirely sounds even less desirable.  If these were just for
humans, then having a schematic name is better than nothing.  But when you
suggest their usage to be "TEs develop their tool to test these GPIOs" that
seems to indicate this becomes ABI and we want stable, documented names, to
limit the churn on users.

I don't believe the gpiochip/pin numbers are considered stable ABI.  Our te=
am
has previously had to do an abstraction between 4.x and 5.x kernel because =
of
changes in that space.

My initial preference would be that you leave them in as schematic names, f=
or
human purposes, until you start using them in code at which point they shou=
ld be
well-documented and using the style we've set out in the document above.

Re: "OpenBMC believes there is only one GPIO to be used to power on the cha=
ssis,
but Bletchley has six."... This does not make it system-specific.  Yosemite=
-v2
has 4 independently managed systems, with their own power sequencing.  There
should be work going on by that team to expand the GPIO documentation to co=
ver N
sub-chassis as well; it just might be that you are ahead of them in documen=
ting
it.

It should be trivial to expand the `power-chassis-control` and
`power-chassis-good` documentation to support sub-chassis.  I can do this f=
or
you if you need.  Many of your GPIOs were related to LEDs which are also al=
ready
covered by this doc (except might need minor wording for sub-chassis as wel=
l).
Can you let me know which other GPIO functions you think you'll need that a=
ren't
already in that document and we can work to get them added?

> > > +&i2c13 {
> > > +	multi-master;
> > > +	aspeed,hw-timeout-ms =3D <1000>;
> > > +	status =3D "okay";
> > > +};
> >=20
> > Was this intentional to have defined a multi-master bus with nothing on=
 it?
> There is a OCP debug card which is a hot plugging device.
> We only need to specify this bus with "multi-mater" property for IPMB sup=
port.

Got it.

--=20
Patrick Williams

--pNxo54Czgw/SMy95
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEBGD9ii4LE9cNbqJBqwNHzC0AwRkFAmGDqCQACgkQqwNHzC0A
wRnlOA//RV+35oOn9wEW+jO6oZi0M5yL51mdYiahoSlrSbjn9M3rydZPeCHA2Nfo
nn+TpAalJ6fNmAjwGdGyQCcorRGgKb4WRw1WW91lErBydFRTSIbL0lJ4dIIQsdjh
VtRhivTCz1UDMsGYGy+rFc4f/NAIXq3fFDaY5ek58BBUZtHMHHpvaQhvEzg982kS
iOSsQ79dInmoX/1m5PQDGt67Fx/lcuJCVmlVl5OFuD3UTmpeSckgbs/kT6S4jiiF
Y/SUWHGOrx9rWwRsNdDgCFJLmWjwTFpopI8hwkw/PkP9w5Y3twv/Ulng4FG/wvuR
eBLc0bYY69bPXfOWP6UVg3ChnJAiE1mNQNlUwNrbvCF/Rc3NtBM4hIOZ6zDLBc6g
l54ITv6DLl07KARnYICr/eVCRvSV2znMHxju3c6Lcsql68T98h3+KGsMzX62dd7Q
O+diXES9DbmR4yMxXN//LN+adaqiRVCo8ubSZ5dnK6bgqJJqd+repyl48XwQCoTU
KES8EeT+s/rye5AG1o877GEt2yoVjdaETlYzZGZTBRO9tKJ1CVrVBs71J1JFaMP0
lCI3rDGGz0vzZjNzVMw+xUO/EH8ddOh9wGBCEzypRNAE5ikipidbms5s2BehMWU/
mPiI/oQk5RHl4q5dxeWi42QScTkXHq3fToJ7ZBoeJN9Oy5Xz4l4=
=cy2B
-----END PGP SIGNATURE-----

--pNxo54Czgw/SMy95--