linux-kernel - Re: [PATCH RESEND] i2c: add sc18is600 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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170618140228.lmrvlgmp5vwnbblv@earth>
Date:   Sun, 18 Jun 2017 16:02:28 +0200
From:   Sebastian Reichel <sre@...nel.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
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@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND] i2c: add sc18is600 driver

Hi Andy,

Thanks for your feedback.

On Sat, Jun 17, 2017 at 09:13:32PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 13, 2017 at 6:47 PM, Sebastian Reichel <sre@...nel.org> wrote:
> > This adds an I²C master driver for SPI -> I²C bus bridge chips.
> > It currently supports NXP's SC18IS600 and SC18IS601, as well as
> > Silicon Labs' CP2120. The driver was only tested on SC18IS600.
> 
> > +static void sc18is600_setup_clock_frequency(struct sc18is600dev *dev)
> > +{
> > +       int reg = DIV_ROUND_UP(dev->clock_base, dev->i2c_clock_frequency);
> > +
> 
> > +       if (reg < 5)
> > +               reg = 5;
> > +       if (reg > 255)
> > +               reg = 255;
> 
> clamp_t()

I will use clamp_val()

> [...]
> lot's of suggestions for improvements.
> [...]

Ack.

> ...and better to use s600dev or alike to avoid confusion.
> 
> > +               return -ENOMEM;
> 
> > +       snprintf(dev->adapter.name, sizeof(dev->adapter.name),
> 
> > +                "SC18IS600 at SPI %02d device %02d",
> 
> Isn't too much for adapter name?
> I don't remember if it's part of ABI, in that case it's even worse.

Well, it's displayed by "i2cdetect -l". Here are the names from some
other drivers, which I used as reference:

i2c-tiny-usb at bus %03d device %03d
i2c-diolan-u2c at bus %03d device %03d
OSIF at bus %03d device %03d
SMBus Via Pro adapter at %04x
SMBus SIS5595 adapter at %04x

> [...]
> lot's of suggestions for improvements.
> [...]

Ack.

-- Sebastian

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