On Fri, Dec 07, 2018 at 10:18:26AM +0200, Vladimir Zapolskiy wrote: > Hi Andy, > > On 12/05/2018 04:11 PM, Andy Shevchenko wrote: > > For the platforms which have no clock provider for the sc16is7xx type of UART, > > introduce an alternative clock-frequency property which would be used instead. > > the subject has a typo in 'clock-frequence', then can you please tell me more, > how is it possible that an SC16IS7xx IC has no clock provider connected to it? I better ask Grigorii about this, since I have no hardware at my possession. > And if there is one, then please just describe it in device tree as well. Tell me how to do this for ACPI? > > > > > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > > Cc: devicetree@xxxxxxxxxxxxxxx > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > --- > > Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt > > index e7921a8e276b..b8cf38a1e43c 100644 > > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt > > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt > > @@ -12,6 +12,8 @@ Required properties: > > - reg: I2C address of the SC16IS7xx device. > > - interrupts: Should contain the UART interrupt > > - clocks: Reference to the IC source clock. > > + OR > > +- clock-frequency: The source clock frequency for the IC. > > > > I strongly dislike this change, I'm inclined to cast a NAK to the series. To be productive, please propose the alternative, otherwise your NAK is nothing to do with a real hardware and approach I proposed. > > 1. 'clock-frequency' is a very specific device tree property, in my opinion > its presence is justified on sort of clock provider devices only (like I2C > controllers), unfortunately the property was added to a number of device > tree bindings improperly, mainly it was done before introduction of > "assigned-clocks" and "assigned-clock-parents" properties in CCF, and then > it was blindly copied. OK, I will wait for your patch to remove such from, for example, 8250_dw.c where same problem had been targeted in the same way. > > 2. SC16IS7xx type of UARTs is a regular clock consumer, ICs always have a valid > clock provider connected to XTAL1 (and XTAL2 in case of a connected > crystal oscillator), thus, if needed, the driver can get input clock rate > by calling standard clk_get_rate(), so the presence of the required 'clocks' > property is sufficient. So what? There is a hardware, there is a clock provider hidden in it. How you would describe it? Platform data? Why? > 3. In some very specific corner cases it might be needed to add another > "assigned-clocks" and "assigned-clock-parents" properties to a particular > device node on a particular board, but their explicit description in device > tree bindings is not needed. Can DT people once in life think outside the box?! -- With Best Regards, Andy Shevchenko
