linux-kernel - Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs' CP2615 Digital Audio Bridge

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]

Message-ID: <20210318055936.GD1053@kunai>
Date:   Thu, 18 Mar 2021 06:59:36 +0100
From:   Wolfram Sang <wsa@...nel.org>
To:     Bence Csókás <bence98@....bme.hu>
Cc:     linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Adding i2c-cp2615: i2c support for Silicon Labs'
 CP2615 Digital Audio Bridge

Hi Bence,

> You are right, sorry, I am still familiarizing myself with `git send-email`

No worries, the progress from your first to your second version was
really great! You can add such information e.g. using the "--annotate"
parameter og git-send-email.

> GPLv2 or later is fine by me. If I change this to "//
> SPDX-License-Identifier: GPL-2.0-or-later", is that OK?

Yes, also perfect.

> > > + * FIXME: There in no quirk flag for specifying that the adapter
> > > + * does not support empty transfers, or that it cannot emit a
> >
> > Can't we use I2C_AQ_NO_ZERO_LEN here?
> 
> I thought that meant the adapter cannot handle NEITHER zero-length
> reads NOR writes, but the CP2615 can do a zero read combined with a
> non-zero write or the other way around, just both cannot be zero. If
> both are zero, the chip just ignores the request, as I've learned from
> a very confusing situation with `i2cdetect`.

I think we still should use I2C_AQ_NO_ZERO_LEN. The almost only use case
is a standalone zero-len read or write. This is not supported by your
adapter as you found out.

> > True! But it makes sense, so we can fix that. We just need to add
> > I2C_AQ_NO_REP_START and a short explanation to i2c.h. If you want, you
> > can do it in a seperate patch. I can do it, too, if you prefer.
> 
> Sure! I should just define it as BIT(7) or something, right? Should I
> do it in a completely different patchset, or is it OK if I submit it
> as the 2/2 of PATCH v3? Are there maybe other adapters that would be

Yes, BIT(7) and same patchset please. But you need to make it 1/2
because you will use it in you driver which is then 2/2.

> affected?

Maybe, but we can fix them later incrementally. Currently, there is also
no client testing the flag, so we have a bit of time here.

> > Maybe skip the defines for VID and PID and use the values directly?
> > I am not a USB expert, not really sure what the consistent way is.
> 
> I think this is how they usually do it, or at least from what I've seen.

Then keep it.

All the best and happy hacking,

   Wolfram


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