Hello Masahiro, This is my understanding of the parameters 'dev' and 'perihp' passed to the function implementing set_state_simple().
On 08.02.2017 04:02, Masahiro Yamada wrote: > Hi. > > > > 2017-02-07 22:30 GMT+09:00 Felix Brack <[email protected]>: > >> + >> +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 know that neither 'dev' nor 'periph' _are_ device tree nodes objects. This is why I use the word 'represent'. > So, you are parsing the "pinctrl-single,pins" property > in the peripheral device, like > > uart: uart { > > pinctrl-single,pins = < > ..... > >; > > }; > > I don't think so (see below). > > > In pinctrl, peripheral nodes should have a phandle to a child of the > pinctrl device. > As you see Linux, the DT should look like this: > > > uart: uart { > > pinctrl-0 = <&uart_pins>; > > }; > > > pinctrl { > > uart_pins: uart_pins { > pinctrl-single,pins = < > .... > >; > }; > > }; > > This is how it works. single_set_state_simple() would be called with 'dev' representing 'pinctrl' node and 'periph' representing 'uart_pins' node. I would have to parse 'periph' for 'pinctrl-single,pins'. > > I use this pin controller on a AM335x based board. Here are the relevant parts from the device tree. >From 'am33xx.dtsi', representing the pin controller itself: am33xx_pinmux: pinmux@800 { compatible = "pinctrl-single"; reg = <0x800 0x238>; ... } >From a a device tree source file, representing the configuration of the pins for the peripheral i2c0: &am33xx_pinmux { pinctrl-names = "default"; pinctrl-0 = <&i2c0_pins>; i2c0_pins: pinmux_i2c0_pins { pinctrl-single,pins = < AM33XX_IOPAD(0x988, PIN_INPUT_PULLUP | MUX_MODE0) /* i2c0_sda.i2c0_sda */ AM33XX_IOPAD(0x98c, PIN_INPUT_PULLUP | MUX_MODE0) /* i2c0_scl.i2c0_scl */ >; }; }; With this, single_set_state_simple() gets called with 'dev' representing 'pinmux@800' and 'periph' representing 'pinmux_i2c0_pins' which makes perfect sense to me. Again I would have to parse 'periph' for 'pinctrl-single,pins'. regards Felix _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
