lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 07 May 2015 13:47:11 +0900 From: Yoshinori Sato <ysato@...rs.sourceforge.jp> To: Arnd Bergmann <arnd@...db.de> Cc: linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org, devicetree@...r.kernel.org Subject: Re: [PATCH v10 19/19] h8300: devicetree source At Mon, 04 May 2015 17:09:21 +0200, Arnd Bergmann wrote: > > On Monday 04 May 2015 19:42:02 Yoshinori Sato wrote: > > + > > + h8intc: intc@0 { > > + compatible = "renesas,h8s-intc", "renesas,h8300-intc"; > > + #interrupt-cells = <1>; > > + interrupt-controller; > > + }; > > The node name should be "interrupt-controller@0", not "intc@0", to follow > the common conventions. OK. > > + tpu: tpu@...fe0 { > > + compatible = "renesas,tpu"; > > + reg = <0xffffe0 16>, <0xfffff0 12>; > > + clocks = <&pclk>; > > + clock-names = "peripheral_clk"; > > + }; > > + > > + timer8: timer@...fb0 { > > + compatible = "renesas,8bit-timer"; > > + reg = <0xffff80 10>; > > + interrupts = <72 75>; > > + clocks = <&pclk>; > > + clock-names = "peripheral_clk"; > > + renesas,mode = <CLOCKEVENTDEVICE>; > > + renesas,div = <DIV_8>; > > + }; > > + > > The renesas,div property seems odd here. How about defining a > "clock-frequency" property and figuring out the divider from > the parent clock in the driver? Hmm... It more better idea. I will fix. > Your new binding makes it mandatory to have a "fclk" clock, which > seems better suited than "peripheral_clk", so I'd suggest you > change the code to match the documentation (rather than the other > way round). Alternatively, you could make this an anonymous > clock and not specify the name at all. OK. > The renesas,mode property seems odd. Why is that needed? It sounds > like you are encoding how you expect the device to be used by > Linux, rather than what it can do in hardware. If you have multiple > variants of the 8bit-timer hardware that have different features, > better use separate compatible strings for them, or a boolean > flag that announces the presence or absence of a feature. This timer have some mode. But driver using only one mode. I think it isn't necessary. > If however, this is just a hint for Linux, maybe you can find a way > for the driver to take a guess itself, e.g. using the first > device it finds as a clockevent device, and only use a clocksource > device if there is more than one? Many clocks are put in order. > > + sci0: serial@...f78 { > > + compatible = "renesas,sci"; > > + reg = <0xffff78 8>; > > + interrupts = <88 89 90 91>; > > + clocks = <&pclk>; > > + clock-names = "peripheral_clk"; > > + }; > > + sci1: serial@...f80 { > > + compatible = "renesas,sci"; > > + reg = <0xffff80 8>; > > + interrupts = <92 93 94 95>; > > + clocks = <&pclk>; > > + clock-names = "peripheral_clk"; > > + }; > > + sci2: serial@...f88 { > > + compatible = "renesas,sci"; > > + reg = <0xffff88 8>; > > + interrupts = <96 97 98 99>; > > + clocks = <&pclk>; > > + clock-names = "peripheral_clk"; > > + }; > > +}; > > The binding for sci requires the clock to be named "sci_ick", so please > use that instead of "peripheral_clk". The driver can handle both. > OK. > Arnd Thanks. -- Yoshinori Sato <ysato@...rs.sourceforge.jp> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists