linux-kernel - Re: [PATCH v2 2/2] i2c: add support for Socionext SynQuacer I2C controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Feb 2018 19:05:59 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Jassi Brar <jassisinghbrar@...il.com>
Subject: Re: [PATCH v2 2/2] i2c: add support for Socionext SynQuacer I2C controller

On Mon, Feb 26, 2018 at 4:58 PM, Ard Biesheuvel
<ard.biesheuvel@...aro.org> wrote:
> On 26 February 2018 at 11:35, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>> On Mon, Feb 26, 2018 at 11:59 AM, Ard Biesheuvel
>> <ard.biesheuvel@...aro.org> wrote:
>>> On 23 February 2018 at 13:12, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>>>> On Fri, Feb 23, 2018 at 2:40 PM, Ard Biesheuvel
>>>> <ard.biesheuvel@...aro.org> 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