linux-kernel - Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

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]

Message-ID: <CAOAejn0Hs7QOgcrHM2t3NsFQeD9dyvThDK-8iM1Lvb7Qv4fGGQ@mail.gmail.com>
Date:   Fri, 17 Mar 2017 14:52:44 +0100
From:   "M'boumba Cedric Madianga" <cedric.madianga@...il.com>
To:     Neil Armstrong <narmstrong@...libre.com>
Cc:     Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Pierre-Yves MORDRET <pierre-yves.mordret@...com>,
        Russell King <linux@...linux.org.uk>,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] i2c: i2c-stm32f7: add driver

Hi,


>> +static int stm32f7_i2c_hw_config(struct stm32f7_i2c_dev *i2c_dev)
>> +{
>> +     struct device_node *of_node = i2c_dev->dev->of_node;
>> +     u32 timing;
>> +     int ret;
>> +
>> +     ret = of_property_read_u32(of_node, "st,i2c-timing", &timing);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Error: missing i2c timing property\n");
>> +             return ret;
>> +     }
>> +
>> +     /* Timing settings */
>> +     writel_relaxed(timing, i2c_dev->base + STM32F7_I2C_TIMINGR);
>
> Hi,
>
> Using a register value in DT is quite ugly since the requirement to calculate the timings
> is quite easy, and well documented.
>
> I wrote it for Zephyr, you can find it here :
> https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/i2c/i2c_stm32lx.c#L31

Thanks for this code. It is very interesting.
With our formula, I just notice that we don't use any i2c rise time or
fall time but they have some impacts in i2c timing computation.
So, the user could not be able to select these values for him use case. Right ?


>
> Another point, maybe you should find a better name for the driver, since this I2C IP is share
> with the STM32Lx also and is not tied to STM32F7.

As far as I know, we don't have any STM32Lx SoC integrated in the
linux kernel mainline.
I choose i2c-stm32f7 name as it is the first Soc integrated in the
linux kernel mainline where this IP could be used.

BR,
Cedric