Hi Uwe, On Wed, 14 Aug 2019 at 15:01, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > Hello Baolin, > > On Wed, Aug 14, 2019 at 09:51:34AM +0800, Baolin Wang wrote: > > On Tue, 13 Aug 2019 at 22:13, Uwe Kleine-König > > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > On Tue, Aug 13, 2019 at 09:46:40PM +0800, Baolin Wang wrote: > > > > +- assigned-clock-parents: The phandle of the parent clock of PWM clock. > > > > > > I'm not sure you need to point out assigned-clocks and > > > assigned-clock-parents as this is general clk stuff. Also I wonder if > > > these should be "required properties". > > > > I think I should describe any properties used by PWM node, like > > 'clocks' and 'clock-names' properties, though they are common clock > > properties. > > Then you might want to describe also "status", "assigned-clock-rates", > "pinctrl-$n", "pinctrl-names", "power-domains", "power-domain-names" and > probably another dozen I'm not aware of. We usually do not describe 'status', but if your device node used "pinctrl-$n", "pinctrl-names" ... common properties, yes, you should describe them to let users know what is the purpose of these properties. That's also asked by DT maintainer Rob. > > > Yes, they are required. Thanks for your comments. > > required in which sense? Why can a Spreadtrum PWM not work when the > clock parents are unspecified? On some Spreadtrum platforms, the default source clock of PWM may not be enabled, so we should force users to select one available source clock for PWM output clock. -- Baolin Wang Best Regards