Re: [PATCH 4/4] i2c: stm32f7: Add SMBus-specific protocols support

From: Wolfram Sang <wsa@kernel.org>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	robh+dt@kernel.org, mark.rutland@arm.com,
	pierre-yves.mordret@st.com, mcoquelin.stm32@gmail.com,
	alexandre.torgue@st.com, linux-i2c@vger.kernel.org,
	devicetree@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, fabrice.gasnier@st.com
Subject: Re: [PATCH 4/4] i2c: stm32f7: Add SMBus-specific protocols support
Date: Tue, 30 Jun 2020 08:40:50 +0200	[thread overview]
Message-ID: <20200630064050.GA996@ninjato> (raw)
In-Reply-To: <20200526103938.GC14423@gnbcxd0016.gnb.st.com>

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

Hi Alain,

> > So, as mentioned in the other review, I'd like to evaluate other
> > possibilities for the above:
> > 
> > - One option is to enable it globally in probe(). Then you lose the
> >   possibility to have a device at address 0x08.
> 
> I'd prefer avoid this solution to not lose the address 0x08.

Understandably.

> > - Enable it in probe() only if there is a generic binding "host-notify".
> 
> Do you mean having the adapter walk through childs node and see if at least
> one of them have the host-notify property ? This mean that such solution
> wouldn't work for device relying on platform data rather than DT nodes.

I meant a generic binding for the host-controller. It could be seen as a
HW description if we need HostNotify on that bus or not.

Maybe it becomes more clear with the R-Car I2C controller as an example.
It only supports one slave address. If I want HostNotify there, I can't
use another slave backend. Now, it could be that I need the slave EEPROM
backend, although there is a HostNotify capable device on the bus. So, I
am leaning to have a generic "host-notify" binding for the host.

I consider platform_data legacy. If we use device_property, we should be
safe regarding all current and future HW descriptions, or?

> > - Let the core scan for a device with HOST_NOTIFY when registering an
> >   adapter and then call back into the driver somehow?
> 
> You mean at adapter registration time only ? Not device probing time ?
> At probing time, we could have the core (i2c_device_probe) check for the flag
> HOST_NOTIFY and if setted call a dedicated host-notify reg callback ?

As said above, I am leaning to the generic property. In addition, it
doesn't feel right to me to add/remove the HostNotify feature at runtime
depending on the client devices. Imagine someone changes another slave
backend to address 0x08 and the HostNotify device comes later. Then, it
won't work all of a sudden.

It feels much safer to me to declare HostNotify as a feature of the IP
core which it either has or it has not, configurable at boot-time.

Makes sense?

Kind regards,

   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