linux-kernel - Re: [PATCH v3 1/1] i2c: algo-pca: Reapply i2c bus settings after reset

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] [day] [month] [year] [list]

Date:   Tue, 8 Sep 2020 09:06:35 +0200
From:   Wolfram Sang <wsa@...nel.org>
To:     Evan Nimmo <evan.nimmo@...iedtelesis.co.nz>
Cc:     andriy.shevchenko@...ux.intel.com, jarkko.nikula@...ux.intel.com,
        jdelvare@...e.de, chris.packham@...iedtelesis.co.nz,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] i2c: algo-pca: Reapply i2c bus settings after
 reset

Hi Evan,

> changes in v2:
> - changed lowercase "pca to uppercase "PCA".
> - reworded and reformatted the multiline comment.
> - moved the clock frequency KERN_INFO closer to the call that sets this.
> - moved the i2c_bus_settings struct to the more generic i2c.h and removed
> - the comments indicating this as being for the pca chip.

As mentioned in v1, I think we should not have it in the generic i2c
headers yet. a) it makes backporting more difficult and b) we need to
find the optimal generic set of parameters which is a seperate issue.

>  
> -		pca_reset(pca_data);
> -
>  		clock = pca_clock(pca_data);
> +
>  		printk(KERN_INFO "%s: Clock frequency is %dkHz\n",
>  		     adap->name, freqs[clock]);

Minor nit: I think there is no need for the extra blank line, but I'll
let you decide.

> +/**
> + * struct i2c_bus_settings - The configured i2c bus settings
> + * @mode: Configured i2c bus mode
> + * @tlow: Configured SCL LOW period
> + * @thi: Configured SCL HIGH period
> + * @clock_freq: The configured clock frequency
> + */
> +struct i2c_bus_settings {

'pca_i2c_bus_settings' or similar?

Rest looks good!

Thanks,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)