On Mon, Feb 26, 2018 at 4:58 PM, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 26 February 2018 at 11:35, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: >> On Mon, Feb 26, 2018 at 11:59 AM, Ard Biesheuvel >> <ard.biesheuvel@xxxxxxxxxx> wrote: >>> On 23 February 2018 at 13:12, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: >>>> On Fri, Feb 23, 2018 at 2:40 PM, Ard Biesheuvel >>>> <ard.biesheuvel@xxxxxxxxxx> wrote: >>>> Replace 'baudclk' with 'pclk' and p->uartclk with i2c->clkrate in >>>> above and you are almost done. >>> I don't think this is better. >> >> It's a pattern over ACPI vs. clk cases at least for now. >> But hold on. We have already an example of dealing with ACPI / >> non-ACPI cases for I2C controllers — i2c-designware-platdrv.c. >> Check how it's done there. >> >> I actually totally forgot about ACPI slaves described in the table. We >> need to take into account the ones with lowest bus speed. >> > > Wow, that code is absolutely terrible. To some degree I may say yes it is. > So even while _DSD device properties require vendor prefixes, which > are lacking in this case, What kind? clock-frequency? Does it require prefix? > and the fact that the ACPI flavor of the > Designware I2C controller now provides two different ways to get the > timing parameters (using device properties or using SSCN/FMCN/etc ACPI > methods), you think this is a shining example of how this should be > implemented? No, those methods because of windows driver and existed ACPI tables at that time. You are not supposed to uglify your case. > Also, I still think implementing a clock device using rate X just to > interrogate it for its rate (returning X) is absolutely pointless. OTOH the deviation in the driver is what I absolutely against of. Driver must not know the resource provider (ideally at all). > So what I can do is invent an ACPI method that returns the PCLK rate. > Would that work for you? Again, looking into existing examples (UART, I2C, etc) we better to create a generic helper in clock framework that would provide us a clock based on property value. But doing different paths for different resource providers is not what we are looking for. P.S. To move this somehow forward I may propose to submit an OF driver, and discuss ACPI part after. -- With Best Regards, Andy Shevchenko