Re: [PATCH v3 0/3] i2c: at91: slave mode support

From: Wolfram Sang <wsa@the-dreams.de>
To: linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	me@jue.yt, nicolas.ferre@microchip.com,
	alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/3] i2c: at91: slave mode support
Date: Tue, 11 Dec 2018 20:28:05 +0100	[thread overview]
Message-ID: <20181211192805.qsxptzkmczmm5vlz@ninjato> (raw)
In-Reply-To: <20180730071443.clib3swtsommj7jk@M43218.corp.atmel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1836 bytes --]

Hi Ludovic,

> > >  - fix checkpatch errors
> > >  - tests:
> > >    - hangs with a SAMA5D4 (master and slave on different busses) after about
> > >    100 transfers. It's the firs time I do this test.
> > >    - some mismatches with a SAMA5D4 as slave and a SAMA5D2 as master
> > >    I don't know if it's a regression. I don't remember having seen this
> > >    behavior previously.
> > >    I think it's worth taking those patches even if there are some possible
> > >    bugs. It'll allow to get more people using it and so to consolidate the
> > >    slave mode support.
> > 
> > I really want to see those patches go upstream, too. But I am also not a
> > big fan of delivering the user something with known issues. Especially
> > not when they affect the main feature to be added. My rationale here is
> > that someone who is able to fix the issues remaining will also be able
> > to pick up and apply patches.
> > 
> > Maybe, maybe if it was to be enabled by a special
> > I2C_AT91_SLAVE_EXPERIMANTEL symbol with lots of explanations. I need to
> > think twice about that, though.
> > 
> 
> I understand your point. The experimental mentionning could be a good
> trade-off. Let me know once you make up your mind.
> 
> > Speaking of Kconfig, I think this series needs to place a
> > 
> > 	select I2C_SLAVE
> > 
> > somewhere.
> > 
> 
> Ok I'll update it if we go further with this set of patches.

Ok, I give in. If you:

* add 'select I2C_SLAVE'
* make slave support selectable by I2C_AT91_SLAVE_STAGING or
  _EXPERIMENTAL or something alike (default n)
* and add to the help text of that symbol the above known issues
  and 'not for production' and 'help wanted' and where to get more
  info and all that

then I'll apply this series soonish. Promised!

Thanks,

   Wolfram

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel