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

[Sagar Kadam]

Hi Bin,

On Thu, Sep 26, 2019 at 7:26 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Sagar,
>
> On Thu, Sep 26, 2019 at 1:54 AM Sagar Kadam <sagar.kadam at sifive.com> wrote:
> >
> > 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?
> >
>
> I think <bank_name>_0 is fine. Thanks!
>
Thanks, I will roll out the next patch with necessary changes.

BR,
Sagar
> Regards,
> Bin