Morning Tomi,

On 2/7/22 18:23, Tomi Valkeinen wrote:

On 07/02/2022 16:38, Vaittinen, Matti wrote:

quoted

Hi again Luca,

On 2/7/22 16:07, Luca Ceresoli wrote:

quoted

Hi Matti,

On 07/02/22 14:21, Vaittinen, Matti wrote:

quoted

Hi dee Ho peeps,

On 2/7/22 14:06, Tomi Valkeinen wrote:

quoted

Hi Luca,

On 06/02/2022 13:59, Luca Ceresoli wrote:

quoted

this RFCv3, codename "FOSDEM Fries", of RFC patches to support the TI
DS90UB9xx serializer/deserializer chipsets with I2C address 
translation.

I am not sure if I am poking in the nest of the wasps - but there's one
major difference with the work I've done and with Toni's / Luca's work.

You are. ;)

quoted

The TI DES drivers (like ub960 driver) packs pretty much everything
under single driver at media/i2c - which (in my opinion) makes the
driver pretty large one.

My approach is/was to utilize MFD - and prepare the regmap + IRQs in 
the
MFD (as is pretty usual) - and parse that much of the device-tree that
we see how many SER devices are there - and that I get the non I2C
related DES<=>SER link parameters set. After that I do kick alive the
separate MFD cells for ATR, pinctrl/GPIO and media.

The ATR driver instantiates the SER I2C devices like Toni's ub960 does.
The SER compatible is once again matched in MFD (for SER) - which again
provides regmap for SER, does initial I2C writes so SER starts
responding to I2C reads and then kicks cells for media and 
pinctrl/gpio.

I believe splitting the functionality to MFD subdevices makes drivers
slightly clearer. You'll get GPIOs/pinctrl under pinctrl as usual,
regmaps/IRQ-chips under MFD and only media/v4l2 related parts under 
media.

There has been quite a fiery discussion about this in the past, you can
grab some popcorn and read
https://lore.kernel.org/linux-media/20181008211205.2900-1-vz@mleia.com/T/#m9b01af81665ac956af3c6d57810239420c3f8cee (local) 


TL;DR: there have been strong opposition the the MFD idea.

Hm. I may be missing something but I didn't see opposition to using MFD
or splitting the drivers. I do see opposition to adding _functionality_
in MFD. If I read this correctly, Lee did oppose adding the I2C stuff,
sysfs attributes etc in MFD. Quoting his reply:

"This driver does too much real work ('stuff') to be an MFD driver.
MFD drivers should not need to care of; links, gates, modes, pixels,
frequencies maps or properties.  Nor should they contain elaborate
sysfs structures to control the aforementioned 'stuff'.

Granted, there may be some code in there which could be appropriate
for an MFD driver.  However most of it needs moving out into a
function driver (or two)."

And I tend to agree with Lee here. I would not put I2C bridge stuff or
sysfs attributes in MFD. But I think it does not mean SERDESes should
not use MFD when they clearly contain more IP blocks than the
video/media ones :) I am confident Lee and others might be much more
welcoming for driver which simply configures regmap and kicks subdriver
for doing the ATR / I2C stuff.

I admit that I don't know MFD drivers too well, but I was thinking about 
this some time back and I wasn't quite sure about using MFD here.

My thinking was that MFD is fine and good when a device contains more or 
less independent functionalities, like a PMIC with, say, gpios and 
regulators, both of which just work as long as the PMIC is powered up.

Here all the functionalities depend on the link (fpdlink or some other 
"link" =), and the serializers. In other words, the link status or any 
changes to the link or the serializers might affect the GPIO/I2C/IRQ 
functionalities.

My use case has been such that once the link between DES &  SER 
established, it should not go away. If it does it is some kind of an 
error and there is no recovery mechanims (at least not yet). Hence I 
haven't prepared full solution how to handle dropping/re-connecting the 
link or re-initializing des/ser/slaves.

So, I don't have any clear concern here. Just a vague feeling that the 
functionalities in this kind of devices may be more tightly tied 
together than in normal MFDs. I could be totally wrong here.

I can't prove you're wrong even if that would be so cool :p

I guess a lot of this boils down how the SER behaves when link is 
dropped. Does it maintain the configuration or reset to some other 
state? And what happens on des & what we need to do in order to reconnect.

My initial feeling is that the DES should always be available as it is 
directly connected to I2C. So DES should always be there.

Access to SERs and the devices on remote buses is naturally depending on 
the link. So dropping the link means access to SERs and remote devices 
start failing - which is probably visible to the MFD sub-devices as 
failing regmap accesses. This needs then appropriate handling.

After that being said, I think we can't get over this problem even when 
not using MFD. As far as I read your code, the SER and DES have 
independent drivers also when MFD is not used. So dropping the link is 
still someting that pulls the legs from the SER, right? I also guess the 
remote I2C devices like sensors are also implemented as independent drivers.

Well, (I hope) I'll see where I end up with my code... It really makes 
this discussion a bit dull when I can't just show the code for 
comparison :/ I don't (yet) see why the MFD approach could not work, and 
I still think it's worth trying - but I now certainly understand why you 
hesitated using MFD. Thanks for taking the time to explain this to me.

Best Regards
	--Matti

-- 
The Linux Kernel guy at ROHM Semiconductors

Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~ this year is the year of a signature writers block ~~