Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1711389 > unrolled thread

Re: [PATCH] i2c: slave-eeprom: Add an OF device ID table

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] i2c: slave-eeprom: Add an OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-08-14 22:00 +0200
    Re: [PATCH] i2c: slave-eeprom: Add an OF device ID table Javier Martinez Canillas <javierm@redhat.com> - 2017-08-16 11:40 +0200
      Re: [PATCH] i2c: slave-eeprom: Add an OF device ID table Wolfram Sang <wsa@the-dreams.de> - 2017-08-16 15:50 +0200

#1711389 — Re: [PATCH] i2c: slave-eeprom: Add an OF device ID table

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 09, 2017 at 12:12:56PM +0200, Javier Martinez Canillas wrote:
> The driver doesn't have a struct of_device_id table but supported devices
> are registered via Device Trees as shown in the following DT binding doc:
> 
> Documentation/devicetree/bindings/i2c/i2c-designware.txt

Uhhh, that needs to be fixed to something else! I don't think i2c slave
functionality should be described in DT. The slave functionality is pure
software, so IMO it doesn't match the "HW description" requirement.

> But this works on the assumption that a I2C device registered via OF will
> always match a legacy I2C device ID and that the MODALIAS reported will
> always be of the form i2c:<device>.
> 
> And this could change in the future so the correct approach is to have an
> OF device ID table if the devices are registered via OF.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks for finding the issue, still NAK to this patch. Are you
interested in updating the docs?

[toc] | [next] | [standalone]

#1712800

Hello Wolfram,

On 08/14/2017 09:52 PM, Wolfram Sang wrote:
> On Wed, Aug 09, 2017 at 12:12:56PM +0200, Javier Martinez Canillas wrote:
>> The driver doesn't have a struct of_device_id table but supported devices
>> are registered via Device Trees as shown in the following DT binding doc:
>>
>> Documentation/devicetree/bindings/i2c/i2c-designware.txt
> 
> Uhhh, that needs to be fixed to something else! I don't think i2c slave
> functionality should be described in DT. The slave functionality is pure
> software, so IMO it doesn't match the "HW description" requirement.
>

Right, indeed.
 
>> But this works on the assumption that a I2C device registered via OF will
>> always match a legacy I2C device ID and that the MODALIAS reported will
>> always be of the form i2c:<device>.
>>
>> And this could change in the future so the correct approach is to have an
>> OF device ID table if the devices are registered via OF.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> Thanks for finding the issue, still NAK to this patch. Are you
> interested in updating the docs?
> 

What kind of change are you looking for? The example was introduced by commit
04606ccc84e3 ("i2c: designware: introducing I2C_SLAVE definitions") that says:

- A example was added to designware-core.txt Documentation that shows
  how the slave can be setup using DTS

So I could change this example to instead use a real EEPROM compatible (e.g:
"microchip,24c02") instead of "linux,slave-24c02". Would that be correct?

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

[toc] | [prev] | [next] | [standalone]

#1712958

[Multipart message — attachments visible in raw view] — view raw

> What kind of change are you looking for? The example was introduced by commit
> 04606ccc84e3 ("i2c: designware: introducing I2C_SLAVE definitions") that says:
> 
> - A example was added to designware-core.txt Documentation that shows
>   how the slave can be setup using DTS

My fault, I missed it :(

> So I could change this example to instead use a real EEPROM compatible (e.g:
> "microchip,24c02") instead of "linux,slave-24c02". Would that be correct?

I think this is Luis call how he'd like this to be changed. Given the
current discussion, I'd think it must be dropped entirely.

Luis?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web