linux-kernel - Re: [PATCH v4 2/3] i2c: add support for Cypress CYUSBS234 USB-I2C adapter

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:	Wed, 3 Dec 2014 12:07:55 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Muthu Mani <muth@...ress.com>
Cc:	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	gregkh@...uxfoundation.org, Johan Hovold <johan@...nel.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-i2c@...r.kernel.org, linux-gpio@...r.kernel.org,
	Rajaram Regupathy <rera@...ress.com>
Subject: Re: [PATCH v4 2/3] i2c: add support for Cypress CYUSBS234 USB-I2C
 adapter

Hi,

thanks for this submission. Looks like an interesting device. However,
some very high level questions first:

> +/*
> + * It exposes sysfs entries under the i2c adapter for getting the i2c transfer
> + * status, reset i2c read/write module, get/set nak and stop bits.
> + */

Yes, I see that. Yet, I don't know why they are needed? The driver
should know when to send NAK/STOP. Why expose that to the user? And why
do we need a reset? Are there stability problems? What about unloading
the module? And what is the use case for reading the status?

> +       if (num > 1) {
> +               dev_err(&adapter->dev, "i2c_msg number is > 1\n");
> +               return -EIO;
> +       }

Ouch! Don't you have any repeated start option somewhere?

Thanks,

   Wolfram

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