* [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree @ 2017-02-07 2:37 Joel Stanley 2017-02-07 19:59 ` Xo Wang 0 siblings, 1 reply; 5+ messages in thread From: Joel Stanley @ 2017-02-07 2:37 UTC (permalink / raw) To: Xo Wang; +Cc: Rick Altherr, openbmc Pin assignments for the EVT-2 board as described in pdbg. The status is disabled so this does not enable the in-kernel FSI driver yet. Signed-off-by: Joel Stanley <joel@jms.id.au> --- Tested on zaius5 with the driver enabled and it worked. arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts index e2195a641414..e1c9b3f4fe44 100644 --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts @@ -74,6 +74,18 @@ gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>; }; }; + + fsi-master { + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio"; + + status = "disabled"; + + clock-gpios = <&gpio ASPEED_GPIO(C, 3) GPIO_ACTIVE_HIGH>; + data-gpios = <&gpio ASPEED_GPIO(C, 2) GPIO_ACTIVE_HIGH>; + trans-gpios = <&gpio ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>; + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>; + mux-gpios = <&gpio ASPEED_GPIO(P, 6) GPIO_ACTIVE_HIGH>; + }; }; &fmc { -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree 2017-02-07 2:37 [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree Joel Stanley @ 2017-02-07 19:59 ` Xo Wang 2017-02-09 2:18 ` Joel Stanley 0 siblings, 1 reply; 5+ messages in thread From: Xo Wang @ 2017-02-07 19:59 UTC (permalink / raw) To: Joel Stanley; +Cc: Rick Altherr, OpenBMC Maillist Hi Joel, Thanks for keeping us up to date. On Mon, Feb 6, 2017 at 6:37 PM, Joel Stanley <joel@jms.id.au> wrote: > Pin assignments for the EVT-2 board as described in pdbg. The status is disabled > so this does not enable the in-kernel FSI driver yet. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > > Tested on zaius5 with the driver enabled and it worked. > > arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts > index e2195a641414..e1c9b3f4fe44 100644 > --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts > @@ -74,6 +74,18 @@ > gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>; > }; > }; > + > + fsi-master { > + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio"; Andrew's comment for Witherspoon applies here as well ("ibm,fsi-master-gpio" makes sense as the "definitive" name). > + > + status = "disabled"; > + > + clock-gpios = <&gpio ASPEED_GPIO(C, 3) GPIO_ACTIVE_HIGH>; > + data-gpios = <&gpio ASPEED_GPIO(C, 2) GPIO_ACTIVE_HIGH>; FYI, we'll return to the Norm-recommended assignments for these at the next board spin to keep FSI pins on their own GPIO port. Not sure how helpful that is in the context of the FSI kernel driver since it's not poking the registers directly. > + trans-gpios = <&gpio ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>; > + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>; > + mux-gpios = <&gpio ASPEED_GPIO(P, 6) GPIO_ACTIVE_HIGH>; > + }; > }; > > &fmc { > -- > 2.11.0 > cheers xo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree 2017-02-07 19:59 ` Xo Wang @ 2017-02-09 2:18 ` Joel Stanley 2017-02-09 19:42 ` Xo Wang 0 siblings, 1 reply; 5+ messages in thread From: Joel Stanley @ 2017-02-09 2:18 UTC (permalink / raw) To: Xo Wang; +Cc: Rick Altherr, OpenBMC Maillist On Wed, Feb 8, 2017 at 6:29 AM, Xo Wang <xow@google.com> wrote: > Hi Joel, > > Thanks for keeping us up to date. > > On Mon, Feb 6, 2017 at 6:37 PM, Joel Stanley <joel@jms.id.au> wrote: >> Pin assignments for the EVT-2 board as described in pdbg. The status is disabled >> so this does not enable the in-kernel FSI driver yet. >> >> Signed-off-by: Joel Stanley <joel@jms.id.au> >> --- >> >> Tested on zaius5 with the driver enabled and it worked. >> >> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >> index e2195a641414..e1c9b3f4fe44 100644 >> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >> @@ -74,6 +74,18 @@ >> gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>; >> }; >> }; >> + >> + fsi-master { >> + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio"; > > Andrew's comment for Witherspoon applies here as well > ("ibm,fsi-master-gpio" makes sense as the "definitive" name). I agree. I had an offline chat with Andrew about it, suggesting that he review the bindings that Chris Bostic has sent upstream. There's already a few issues identified with them that we will have to clean up. I suggest leave this matching the other systems for now, and when we settle on the new binding I will migrated all machines to use that. > >> + >> + status = "disabled"; >> + >> + clock-gpios = <&gpio ASPEED_GPIO(C, 3) GPIO_ACTIVE_HIGH>; >> + data-gpios = <&gpio ASPEED_GPIO(C, 2) GPIO_ACTIVE_HIGH>; > > FYI, we'll return to the Norm-recommended assignments for these at the > next board spin to keep FSI pins on their own GPIO port. Not sure how > helpful that is in the context of the FSI kernel driver since it's not > poking the registers directly. I assume there are electrical reasons for doing that. How do you suggest we handle the old and new device tree? We could detect the board revision and then fix up the device tree in u-boot. > >> + trans-gpios = <&gpio ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>; >> + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>; >> + mux-gpios = <&gpio ASPEED_GPIO(P, 6) GPIO_ACTIVE_HIGH>; >> + }; >> }; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree 2017-02-09 2:18 ` Joel Stanley @ 2017-02-09 19:42 ` Xo Wang 2017-02-13 2:07 ` Joel Stanley 0 siblings, 1 reply; 5+ messages in thread From: Xo Wang @ 2017-02-09 19:42 UTC (permalink / raw) To: Joel Stanley; +Cc: Rick Altherr, OpenBMC Maillist On Wed, Feb 8, 2017 at 6:18 PM, Joel Stanley <joel@jms.id.au> wrote: > On Wed, Feb 8, 2017 at 6:29 AM, Xo Wang <xow@google.com> wrote: >> Hi Joel, >> >> Thanks for keeping us up to date. >> >> On Mon, Feb 6, 2017 at 6:37 PM, Joel Stanley <joel@jms.id.au> wrote: >>> Pin assignments for the EVT-2 board as described in pdbg. The status is disabled >>> so this does not enable the in-kernel FSI driver yet. >>> >>> Signed-off-by: Joel Stanley <joel@jms.id.au> >>> --- >>> >>> Tested on zaius5 with the driver enabled and it worked. >>> >>> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>> index e2195a641414..e1c9b3f4fe44 100644 >>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>> @@ -74,6 +74,18 @@ >>> gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>; >>> }; >>> }; >>> + >>> + fsi-master { >>> + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio"; >> >> Andrew's comment for Witherspoon applies here as well >> ("ibm,fsi-master-gpio" makes sense as the "definitive" name). > > I agree. I had an offline chat with Andrew about it, suggesting that > he review the bindings that Chris Bostic has sent upstream. There's > already a few issues identified with them that we will have to clean > up. > > I suggest leave this matching the other systems for now, and when we > settle on the new binding I will migrated all machines to use that. > Reviewed-by: Xo Wang <xow@google.com> >> >>> + >>> + status = "disabled"; >>> + >>> + clock-gpios = <&gpio ASPEED_GPIO(C, 3) GPIO_ACTIVE_HIGH>; >>> + data-gpios = <&gpio ASPEED_GPIO(C, 2) GPIO_ACTIVE_HIGH>; >> >> FYI, we'll return to the Norm-recommended assignments for these at the >> next board spin to keep FSI pins on their own GPIO port. Not sure how >> helpful that is in the context of the FSI kernel driver since it's not >> poking the registers directly. > > I assume there are electrical reasons for doing that. > Not that I know of. Somebody was thinking that moving all the FSI pins to their own GPIO port(s) would allow a single byte or word write to the output data register to set pin states without read-modify-write. I don't think that's an optimization we can apply through gpiolib for the kernel FSI driver. Hence my skepticism about helpfulness. > How do you suggest we handle the old and new device tree? > > We could detect the board revision and then fix up the device tree in u-boot. > Yeah, sounds good. Having board revision detection and device tree fixups in U-boot seems useful in general. >> >>> + trans-gpios = <&gpio ASPEED_GPIO(O, 6) GPIO_ACTIVE_HIGH>; >>> + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>; >>> + mux-gpios = <&gpio ASPEED_GPIO(P, 6) GPIO_ACTIVE_HIGH>; >>> + }; >>> }; cheers xo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree 2017-02-09 19:42 ` Xo Wang @ 2017-02-13 2:07 ` Joel Stanley 0 siblings, 0 replies; 5+ messages in thread From: Joel Stanley @ 2017-02-13 2:07 UTC (permalink / raw) To: Xo Wang; +Cc: Rick Altherr, OpenBMC Maillist On Fri, Feb 10, 2017 at 6:12 AM, Xo Wang <xow@google.com> wrote: > On Wed, Feb 8, 2017 at 6:18 PM, Joel Stanley <joel@jms.id.au> wrote: >> On Wed, Feb 8, 2017 at 6:29 AM, Xo Wang <xow@google.com> wrote: >>> Hi Joel, >>> >>> Thanks for keeping us up to date. >>> >>> On Mon, Feb 6, 2017 at 6:37 PM, Joel Stanley <joel@jms.id.au> wrote: >>>> Pin assignments for the EVT-2 board as described in pdbg. The status is disabled >>>> so this does not enable the in-kernel FSI driver yet. >>>> >>>> Signed-off-by: Joel Stanley <joel@jms.id.au> >>>> --- >>>> >>>> Tested on zaius5 with the driver enabled and it worked. >>>> >>>> arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 12 ++++++++++++ >>>> 1 file changed, 12 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>>> index e2195a641414..e1c9b3f4fe44 100644 >>>> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>>> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts >>>> @@ -74,6 +74,18 @@ >>>> gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_LOW>; >>>> }; >>>> }; >>>> + >>>> + fsi-master { >>>> + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio"; >>> >>> Andrew's comment for Witherspoon applies here as well >>> ("ibm,fsi-master-gpio" makes sense as the "definitive" name). >> >> I agree. I had an offline chat with Andrew about it, suggesting that >> he review the bindings that Chris Bostic has sent upstream. There's >> already a few issues identified with them that we will have to clean >> up. >> >> I suggest leave this matching the other systems for now, and when we >> settle on the new binding I will migrated all machines to use that. >> > > Reviewed-by: Xo Wang <xow@google.com> Applied to dev-4.7. >>> >>> FYI, we'll return to the Norm-recommended assignments for these at the >>> next board spin to keep FSI pins on their own GPIO port. Not sure how >>> helpful that is in the context of the FSI kernel driver since it's not >>> poking the registers directly. >> >> I assume there are electrical reasons for doing that. >> > > Not that I know of. Somebody was thinking that moving all the FSI pins > to their own GPIO port(s) would allow a single byte or word write to > the output data register to set pin states without read-modify-write. > > I don't think that's an optimization we can apply through gpiolib for > the kernel FSI driver. Hence my skepticism about helpfulness. Doing that in the kernel would be quite tricky. >> How do you suggest we handle the old and new device tree? >> >> We could detect the board revision and then fix up the device tree in u-boot. >> > > Yeah, sounds good. Having board revision detection and device tree > fixups in U-boot seems useful in general. Great. It would be good to move some of the rev-specific code you have in arch/arm/mach-aspeed/aspeed.c into the kernel. We are inching closer to being able to boot a host without any of those workarounds. Cheers, Joel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-13 2:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-07 2:37 [PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree Joel Stanley 2017-02-07 19:59 ` Xo Wang 2017-02-09 2:18 ` Joel Stanley 2017-02-09 19:42 ` Xo Wang 2017-02-13 2:07 ` Joel Stanley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.