[U-Boot] [PATCH v2] Add single register pin controller driver

[Felix Brack]

Hello Masahiro,

On 08.02.2017 16:27, Masahiro Yamada wrote:
> Hi.
> 
> 
>>> 2017-02-07 22:30 GMT+09:00 Felix Brack <fb at ltec.ch>:
>>>
>>>> +
>>>> +static int single_set_state_simple(struct udevice *dev,
>>>> +                                  struct udevice *periph)
>>>> +{
>>>> +       const void *fdt = gd->fdt_blob;
>>>> +       const struct single_fdt_pin_cfg *prop;
>>>> +       int len;
>>>> +
>>>> +       prop = fdt_getprop(fdt, periph->of_offset, "pinctrl-single,pins", &len);
>>>
>>>
>>> This seems wrong to me.
>>>
>>>
>>> The "periph" is a peripheral device (like UART, eMMC, USB, etc.).
>>>
>>
>> In the case above 'dev' represents the pin controller node of the DT and
>> 'periph' represents the DT node holding the pin configuration
>> information for a specific peripheral like i2c0, not the peripheral
>> itself.
> 
> 
> I recommend you to read pinctrl-uclass.c once again.
>

I did.

> 
> Did you track how "periph" was passed across the following function calls?
> 

I did.

> device_probe()

>From 'pinctrl-uclass.c::device_probe()':

---
/*
 * Process pinctrl for everything except the root device, and
 * continue regardless of the result of pinctrl. Don't process pinctrl
 * settings for pinctrl devices since the device may not yet be
 * probed.
 */
if (dev->parent && device_get_uclass_id(dev) != UCLASS_PINCTRL)
	pinctrl_select_state(dev, "default");
---

The comment says it all. This is where 'periph' originates.

>   -> pinctrl_select_state()
>       -> pinctrl_select_state_simple()
>           -> ops->set_state_simple()
> 
> 

Also the outcome reported by 'dm tree' makes sense to me:

 Class       Probed   Name
----------------------------------------
 root        [ + ]    root_driver
 simple_bus  [ + ]    `-- ocp
 simple_bus  [ + ]        |-- l4_wkup at 44c00000
 simple_bus  [ + ]        |   `-- scm at 210000
 pinctrl     [ + ]        |       `-- pinmux at 800
 pinconfig   [ + ]        |           |-- pinmux_i2c0_pins
 pinconfig   [ + ]        |           |-- pinmux_i2c1_pins
 pinconfig   [   ]        |           |-- pinmux_i2c2_pins
 pinconfig   [ + ]        |           |-- pinmux_spi1_pins
 pinconfig   [   ]        |           |-- pinmux_uart0_pins
 pinconfig   [   ]        |           |-- pinmux_uart1_pins
 pinconfig   [ + ]        |           |-- pinmux_uart3_pins
 pinconfig   [ + ]        |           |-- pinmux_clkout2_pin
 pinconfig   [   ]        |           |-- cpsw_default
 pinconfig   [   ]        |           |-- davinci_mdio_default
 pinconfig   [ + ]        |           |-- pinmux_mmc1_pins
 pinconfig   [ + ]        |           |-- pinmux_mmc2_pins
 pinconfig   [   ]        |           |-- lcd_pins_s0
 pinconfig   [   ]        |           `-- pinmux_dcan0_pins
 serial      [   ]        |-- serial at 44e09000
 serial      [   ]        |-- serial at 48022000
 serial      [ + ]        |-- serial at 481a6000
 i2c         [   ]        |-- i2c at 44e0b000
 i2c         [ + ]        |-- i2c at 4802a000
 i2c_generic [ + ]        |   |-- generic_60
 i2c_generic [ + ]        |   `-- generic_50
 i2c         [   ]        `-- i2c at 4819c000

regards Felix