Re: [PATCH v2] of: i2c: Add idle-disconnect DT property to PCA954x mux driver

[Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>]

Hi Wolfram,

On Thursday 15 January 2015 14:19:35 Wolfram Sang wrote:
> On Thu, Jan 15, 2015 at 02:09:09PM +0100, Alexander Sverdlin wrote:
> > On 15/01/15 13:32, ext Wolfram Sang wrote:
> >> On Fri, Dec 19, 2014 at 06:00:10PM +0100, Alexander Sverdlin wrote:
> >>> of: i2c: Add idle-disconnect DT property to PCA954x mux driver
> >>> 
> >>> Add idle-disconnect device tree property to PCA954x mux driver. The new
> >>> property forces the multiplexer to disconnect child buses in idle
> >>> state. This is used, for example, when there are several multiplexers
> >>> on the same bus and the devices on the underlying buses might have
> >>> same I2C addresses.
> >> 
> >> Basically OK. Question to DT maintainers: "idle-disconnect",
> >> "i2c-mux-idle-disconnect", or is there another existing binding we could
> >> use?
> >> 
> >>> At the same time old (and not used in the tree) platform data binding
> >>> deselect_on_exit is removed to simplify the implementation. Old binding
> >>> has different (per-channel) semantics and doesn't fit well in the new
> >>> concept.
> >> 
> >> I'd prefer to keep it. It should be only one || more. It is not really
> >> in the way IMO.
> > 
> > It complicates the implementation 3x times :) This is part of our
> > discussion with Laurent:
>
> Does it? I don't want DT and platform_data to behave equally. I just
> want to keep being backwards compatible. So, I'd suggest:
> 
> (pdata && pdata->modes[num].deselect_on_exit) || idle_disconnect ?
> pca954x_deselect_mux : NULL);
>
> >> I'm not keen to brake out-of-tree code (if any), but may be it will be
> >> decided to drop this per-channel deselect_on_exit, because it's not used
> >> at least in the kernel tree...
> 
> I couldn't find a user of the platform_data, at all. But removing
> platform_data support is a seperate patch, and deprecating platform_data
> is a seperate and general issue IMO.

Sure, but it could be a preliminary patch on top of which to add idle-
disconnect DT support :-)

-- 
Regards,

Laurent Pinchart