[U-Boot] [U-BOOT PATCH v1 0/2] add gpio support for HiFive Unleashed A00 board.

[Sagar Kadam]

Hi Bin,

On Wed, Sep 18, 2019 at 1:23 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Sagar,
>
> On Tue, Sep 10, 2019 at 11:44 PM Sagar Shrikant Kadam
> <sagar.kadam at sifive.com> wrote:
> >
> > U-Boot currently is missing GPIO support for FU540-C000 SoC which is
> > mounted on HiFive Unleashed A00 board. This patch is intended to add DM
> > based GPIO controller driver in order to access GPIO pins within the SoC
> > using GPIO command in U-Boot. More details on the GPIO controller within
> > the SoC can be found at[1]
> >
> > The driver is based above master branch of u-boot-riscv.git and provides a
> > method to configure Input/Output mode of the GPIO pin along with an option
> > to set or clear state of the GPIO pin. The patch is available in
> > dev/sagark/gpio_v3 branch here[2].
> >
> > GPIO device node added to the mainline bound device tree for HiFive
> > Unleashed is available in dev/sagark/mlv5.3-rc5 branch of repo here[3].
> >
> > This implementation is ported from linux driver submitted for review
> > at [4].
> >
> > More details of GPIO pin routing on J1 header is available in schematic
> > document[5]
> >
> > [1] https://static.dev.sifive.com/FU540-C000-v1.0.pdf
> > [2] https://github.com/sagsifive/u-boot
> > [3] https://github.com/sagsifive/riscv-linux-hifive/
> > [4] https://lkml.org/lkml/2018/10/9/1103
> > [5] https://static.dev.sifive.com/dev-kits/hifive-unleashed/hifive-unleashed-a00-schematics.pdf
> >
> > Driver Testing:
> > #Set GPIO1 high.
> > =>gpio set 1
> >   Can be confirmed by probing pin No #24 on J1 Header or memory dump of
> >   gpio register space viz: #md 0x10060000
> >
> > #Set GPIO1 low
> > =>gpio clear 0
> >
> > #Toggle GPIO1
> > =>gpio toggle 1         #Toggle value of GPIO1
> > =>gpio toggle 1         #Toggle value of GPIO1
> >
> > #Configure pin as input
> > =>gpio input 3          #Configure gpio line 3 as input.
> >
> > #Error check
> > =>gpio set 16           #Not a valid GPIO number for FU540-C000
> >   GPIO: '16' not found
> >   Command 'gpio' failed: Error -22
> >
>
> I tested this:
>
> => gpio status -a
> Bank gpio at 10060000:
> gpio at 100600000: unknown
> gpio at 100600001: unknown
> gpio at 100600002: unknown
> gpio at 100600003: unknown
> gpio at 100600004: unknown
> gpio at 100600005: unknown
> gpio at 100600006: unknown
> gpio at 100600007: unknown
> gpio at 100600008: unknown
> gpio at 100600009: unknown
> gpio at 1006000010: unknown
> gpio at 1006000011: unknown
> gpio at 1006000012: unknown
> gpio at 1006000013: unknown
> gpio at 1006000014: unknown
> gpio at 1006000015: unknown
>
> The status is "unknown" for all gpio pins, which is wrong. It should
> be either input or output.

Thank you for your suggestions.
The get_function operation is missing for this driver and so the
status is unknown.
I will implement it and send a revised version. Thanks for catching this.
Please correct me if I am wrong, what I do see is that the gpio command
uses the bank name appended before the GPIO number. So the bank_name
as assigned in the driver probe function gets prefixed to the pin number and
so it shows:
gpio at 100600000
gpio at 100600001
and so on.
I see that few driver's updates the uc_priv->bank_name in probe function
with '_' as the separator between bank_name and pin number and so
#gpio status -a will show it as :

Bank <bank_name>:
<bank_name>_0: input : 1 []
<bank_name>_1: input : 1 []
........ and so on

eg: In the current case here it will show as
Bank gpio at 10060000_:
gpio at 10060000_0
gpio at 10060000_1 and so on.

Please let me know if this implementation is ok.
>
> Also the gpio pin name is weird. I think we should use "0, 1, 2 ..."
>
The current implementation of the gpio_get_status function includes
the base_name
to the pin description. Truncating it here can help to get pin numbers
as just numbers
"0,1,2....". I will also include this if needed?

Thanks & BR,
Sagar Kadam

> Regards,
> Bin
>
> Regards,
> Bin