Franklin, On 07/26/2017 08:27 AM, Franklin S Cooper Jr wrote: > > On 07/26/2017 08:00 AM, Suman Anna wrote: >> Hi Keerthy, >> >> On 07/26/2017 01:45 AM, Keerthy wrote: >>> The patch adds keystone-k2g compatible, specific properties and >>> an example. > > Seems we are adding information regarding several Keystone 2 SoCs. So > the commit and subject should be tweaked to reflect this. >> >> Please update patch header to "dt-bindings: gpio: davinci: ...." >> >>> >>> Signed-off-by: Keerthy <j-keerthy@xxxxxx> >>> --- >>> >>> Changes in v3: >>> >>> * Added details about family of SoCs corresponding to compatibles. >>> >>> .../devicetree/bindings/gpio/gpio-davinci.txt | 40 +++++++++++++++++++++- >>> 1 file changed, 39 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> index 5079ba7..fb9efee 100644 >>> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> @@ -1,7 +1,10 @@ >>> Davinci/Keystone GPIO controller bindings >>> >>> Required Properties: >>> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio" >>> +- compatible: should be "ti,dm6441-gpio": for Davinci da850 SoCs >>> + "ti,keystone-gpio": for Keystone 2 66AK2H/K, 66AK2L, >>> + 66AK2E SoCs >>> + "ti,keystone-k2g-gpio", "ti,keystone-gpio": for 66AK2G >>> >>> - reg: Physical base address of the controller and the size of memory mapped >>> registers. >>> @@ -26,6 +29,17 @@ The GPIO controller also acts as an interrupt controller. It uses the default >>> two cells specifier as described in Documentation/devicetree/bindings/ >>> interrupt-controller/interrupts.txt. >>> >>> +Required Properties specific to keystone-k2g >> >> Thanks for updating the binding for the clocks, but clocks are not >> specific to K2G. They also apply to other K2 SoCs. Davinci platforms do >> not have DT clocks atm, so the Davinci portion can be updated later if >> and when they get added. >> > > What about power-domain property? > >>> + >>> +- clocks: Should contain devices input clock. The first parameter >>> + is a handle to k2g_clks. The second parameter is the >>> + device ID and the third parameter is the clock ID. One can >>> + refer: http://processors.wiki.ti.com/index.php/TISCI#66AK2G02_Data >> >> No need for this link here, just refer to the appropriate clock >> bindings. Some thing like the following would be better > > This information is helpful especially with the macros being dropped. > However, I agree that this is not the place for this information. > Probably should be linked to in the ti,sci-clk.txt and > sci-pm-domain.txt. However, both of these are outdated since it is > referring to macros and includes that don't exist or will no longer exist. You are probably looking at the bindings on current master. These have already been addressed/updated, lookup linux-next. regards Suman > >> >> - clocks: Should contain the device's input clock, and >> should be defined as per the appropriate clock >> bindings consumer usage in, >> >> Documentation/devicetree/bindings/clock/keystone-gate.txt >> for 66AK2HK/66AK2L/66AK2E SoCs or, >> >> Documentation/devicetree/bindings/clock/ti,sci-clk.txt >> for 66AK2G SoCs >> >>> + >>> + Example: <&k2g_clks 0x001c 0x0>; >> >> This can be dropped as well, below example already demonstrates it. >> >>> + >>> +- clock-names: The driver expects the clock name to be "gpio"; >> >> Just say, Should be "gpio". No need of mentioning about the driver. >> >>> + >>> Example: >>> >>> gpio: gpio@1e26000 { >>> @@ -60,3 +74,27 @@ leds { >>> ... >>> }; >>> }; >>> + >>> +Example for keystone-k2g: >> >> s/keystone-k2g/66AK2G/ >> >>> + >>> +gpio0: gpio@2603000 { >>> + compatible = "ti,keystone-k2g-gpio", "ti,keystone-gpio"; >>> + reg = <0x02603000 0x100>; >>> + gpio-controller; >>> + #gpio-cells = <2>; >>> + interrupts = <GIC_SPI 432 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 433 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 434 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 435 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 436 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 437 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 438 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 439 IRQ_TYPE_EDGE_RISING>, >>> + <GIC_SPI 440 IRQ_TYPE_EDGE_RISING>; >>> + interrupt-controller; >>> + #interrupt-cells = <2>; >>> + ti,ngpio = <144>; >>> + ti,davinci-gpio-unbanked = <0>; >>> + clocks = <&k2g_clks 0x001b 0x0>; >>> + clock-names = "gpio"; >>> +}; > > If your going to talk about other Keystone 2 devices it would be helpful > to include an example for one of them since they have slightly different > properties. >>> >> >> regards >> Suman >> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
