[PATCH] ARM: dts: aspeed: Add FSI pins to Zaius device tree

[Xo Wang]

On Wed, Feb 8, 2017 at 6:18 PM, Joel Stanley <joel at jms.id.au> wrote:
> On Wed, Feb 8, 2017 at 6:29 AM, Xo Wang <xow at google.com> wrote:
>> Hi Joel,
>>
>> Thanks for keeping us up to date.
>>
>> On Mon, Feb 6, 2017 at 6:37 PM, Joel Stanley <joel at 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 at 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 at 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