[U-Boot] [PATCH 01/10] rockchip: rk3288: Add pinctrl support for the gmac ethernet interface

[Simon Glass]

Hi Sjoerd,

On 9 October 2015 at 10:51, Sjoerd Simons <sjoerd.simons at collabora.co.uk> wrote:
> On Fri, 2015-10-09 at 10:36 +0100, Simon Glass wrote:
>> Hi Sjoerd,
>>
>> On 5 October 2015 at 10:04, Sjoerd Simons <
>> sjoerd.simons at collabora.co.uk> wrote:
>> > On Sat, 2015-10-03 at 15:29 +0100, Simon Glass wrote:
>> > > > On 1 October 2015 at 10:48, Sjoerd Simons <
>> > > > sjoerd.simons at collabora.co.uk> wrote:Add support for the gmac
>> > > > ethernet interface to pinctrl. This hardcodes
>> >
>> > > > the setup to match that of the firefly and Radxa Rock2 boards,
>> > > > using the
>> > > > RGMII phy mode for gmac interface and GPIO4B0 as the phy reset
>> > > > GPIO.
>> > > >
>> > > > Signed-off-by: Sjoerd Simons <sjoerd.simons at collabora.co.uk>
>> > > > ---
>> > > >
>> > > >  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228
>> > > > ++++++++++++++++++++++++
>> > > >  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
>> > > >  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102
>> > > > +++++++++++
>> > > >  3 files changed, 331 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > index 0117a17..b7dda47 100644
>> > > > --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > +               /* Assuming GPIO4B0_GPIO is phy-reset*/
>> > >
>> > > Space before */
>> > > +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
>> > >
>> > > Would this normally be handled by a GPIO? Does it belong in
>> > > pinmux?
>> >
>> > Hrm, i'm changing the wrong pin there aren't I, woops (well either
>> > that
>> > or the comment is wrong)? Anyway, yes the PHY reset is typically a
>> > GPIO, this should just set up the respective pin in GPIO mode which
>> > is
>> > the job of pinmuxing right ? :)
>> >
>> > Or maybe i'm misunderstanding your comment here?
>>
>> I would hope that this would use gpio_request_by_name() instead, with
>> the GPIO specified in the device tree. See for example:
>
>
>> vcc_sd: sdmmc-regulator {
>>    compatible = "regulator-fixed";
>>    gpio = <&gpio7 11 GPIO_ACTIVE_LOW>;
>>    ...
>> };
>
> Sure and _that_ part is done via:
>   snps,reset-gpio = <&gpio4 8 GPIO_ACTIVE_LOW>;
>
> Which gets handled by the core designware driver in my patchset.
>
> What this code is doing the equivalent of (in your example):
>
> vcc_sd: sdmmc-regulator {
>   ...
>   pinctrl-names = "default";
>   pinctrl-0 = <&sdmmc_pwr>;
>   ...
> };
>
> sdmmc_pwr: sdmmc-pwr {
>      rockchip,pins = <7 11 RK_FUNC_GPIO &pcfg_pull_none>;
> };
>
> In other words, it sets the function of the respective pin to be used
> for GPIO. However it doesn't do any control of it.

I see. In that case I suspect we should create a full pinctrl driver
for Rockchip. The framework is there - Masahiro has sent patches for
an implementation for Uniphier.

Regards,
Simon