Re: [PATCH 0/1] add devm_of_clk_get() and devm_of_clk_get_by_name() functions

From: Paul Osmialowski
Date: Tue Sep 29 2015 - 00:46:32 EST


Hi Stephen,

Thanks for all of your comments.

On Mon, 28 Sep 2015, Stephen Boyd wrote:

> I'd say your binding is wrong. Either the container node
> "pinctrl" is a software concept that contains the two devices for
> port_a and port_b or there's only one pinctrl device that happens
> to span some number of 0x1000 size banks. The former would be
> written as so
>
> pinctrl {
> compatible = "fsl,kenetis-pinctrl";
> reg = <0x40049000 0x2000>;
> clocks = <&sim SIM_CLK_SCGC5_PORTA>, <&sim SIM_CLK_SCGC5_PORTB>;
> };
>

I tried this and actually this looks similar to my first approach to this
driver. I wasn't happy with the fact that pin banks are so loosely coupled
with resources they use.

I thought that making pin controller a container for pin bank devices
would create better coupling and simply look better in DT file.

With this first approach example pin definition would look like:

fsl,kinetis-pins = <PORT_A 0 1 &pcfg_pull_pin_default> ...

where the binding format is fsl,kinetis-pins = <bank pin function CONFIG>

&pcfg_pull_pin_default is defined as:

pcfg_pull_pin_default: pcfg-pull-pin-default {
bias-pull-pin-default;
};

...and PORT_A would have to be defined as preprocessor macro in some
header file:

#define PORT_A 0
#define PORT_B 1
#define PORT_C 2
#define PORT_D 3
#define PORT_E 4
#define PORT_F 5
#define PORT_NUM 6

That's another thing I'd want to avoid.

I wanted a DT file which driver could use to figure out how many pin banks
there are, what clocks and IO ranges they use and how pins are associated
with banks.

Now I see I pasted example from some old file (sorry for that), it differs
in one small detail, so again:

pinctrl: pinctrl {
compatible = "fsl,kinetis-pinctrl";
#address-cells = <1>;
#size-cells = <1>;
ranges;

port_a: pin-bank@40049000 {
compatible = "fsl,kinetis-pin-bank";
reg = <0x40049000 0x1000>;
clocks = <&sim SIM_CLK_SCGC5_PORTA>;
};

port_b: pin-bank@4004a000 {
compatible = "fsl,kinetis-pin-bank";
reg = <0x4004a000 0x1000>;
clocks = <&sim SIM_CLK_SCGC5_PORTB>;
};
...
};

Now, assuming use of of_find_node_by_phandle(), example pin definition
would look like:

fsl,kinetis-pins = <&port_a 0 1 &pcfg_pull_pin_default> ...

Things are getting connected together, no preprocessor definitions, no
extra header file. What can be wrong with this design?

I'll prepare second iteration with updated documentation (and other
smaller cleanups) soon.

Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/