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

[Joel Stanley]

On Fri, Feb 10, 2017 at 6:12 AM, Xo Wang <xow at google.com> wrote:
> 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>

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