linux-kernel - Re: [PATCH v3] i2c: rk3x: fix bug that cause measured high

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, 8 Dec 2014 10:53:44 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Wolfram Sang <wsa@...-dreams.de>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>, Addy Ke <addy.ke@...k-chips.com>,
	Max Schwarz <max.schwarz@...ine.de>,
	Heiko Stübner <heiko@...ech.de>,
	Olof Johansson <olof@...om.net>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Tao Huang <huangtao@...k-chips.com>,
	Lin Huang <hl@...k-chips.com>,
	姚智情 <yzq@...k-chips.com>,
	Chris <zyw@...k-chips.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kever Yang <kever.yang@...k-chips.com>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Jianqun Xu <xjq@...k-chips.com>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	caesar <caesar.wang@...k-chips.com>,
	Eddie Cai <cf@...k-chips.com>, Han Jiang <hj@...k-chips.com>,
	Shunqian Zheng <zhengsq@...k-chips.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3] i2c: rk3x: fix bug that cause measured high_ns doesn't
 meet I2C spec

Hi,

On Mon, Dec 8, 2014 at 9:34 AM, Wolfram Sang <wsa@...-dreams.de> wrote:
>
>> > There are different capitalisation of i2c in the patch and the commit log. I
>> > don't know what Wolfram prefers here, but using the same spelling
>> > everywhere would be nice.
>>
>> Can you please point out?  IIRC you should always capitalize I2C in
>> prose (descriptions, comments, documentation, etc).  However when used
>> in places which specific capitalization standards it should be
>> lowercase.  That means file names, directory names, device tree
>> property names, etc should be lower case.
>
> This is what I prefer, too. However, I am not too strict about it.
> I mean, both are readable.

Glad my understanding is OK.  :)

>
>> >> +     /* Read rise and fall ns; if not there use the default max from spec */
>> >> +     if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-rising-time-ns",
>> >> +                              &i2c->rise_ns)) {
>> >> +             if (i2c->scl_frequency <= 100000)
>> >> +                     i2c->rise_ns = 1000;
>> >> +             else
>> >> +                     i2c->rise_ns = 300;
>> >> +     }
>> >> +     if (of_property_read_u32(pdev->dev.of_node, "i2c-scl-falling-time-ns",
>> >> +                              &i2c->fall_ns))
>> >> +             i2c->fall_ns = 300;
>> >> +
>> > I don't know if other drivers do the same (I assume they should). If so,
>> > moving this logic into the core would be nice. I guess this can still be
>> > done later.
>
> Let's leave that for later. If we do it, we should start with bus speed
> setting first, probably.

OK, great.


So just to summarize for Addy, I think you're being asked to spin one more time.

1. Update the patch description as per Uwe.  Fix other typos pointed out by him.

2. Since you're spinning anyway, adjust "i2c" capitalization to "I2C".

3. If you agree with Uwe, rename spec_max_data_hold_ns to
spec_data_valid_ns.  If not, we should debate in a followup patch.

4. If you think spec_data_hold_buffer_ns could be better represented
in another way (should this be i2c-sda-falling-time-ns"?), please do
so.  If not, we should debate in a followup patch.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/