Re: [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram,

On 12/19/24 14:55, Wolfram Sang wrote:
Introduce I3C support by defining I3C accessors for regmap and
implementing an I3C driver. Enable I3C for the NXP P3T1755.

Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
---

Two things I would like to discuss:

- Shall we reuse i2c_device_id's to get a proper name for the chip?
   Looks strange, but adding all the info again for I3C looks strange as
   well


I don't think there will be many i3c devices supporting LM75 compatible chips.
For the few i3c chips which do happen do be lm75 compatible, we should have
something like

struct lm75_i3c_devices {
	enum lm75_type type;
	const char *name;
};

with an instance for each i3c device, and then point i3c_device_id->data to it.
And I think "lm75compatible" is really a terrible hwmon device name ;-).

FWIW, it is too bad that i3c_device_id doesn't have a "name" field. That would
really come handy here.

An alternative would be to just use dev_name(i3cdev_to_dev(i3cdev)),
but that would not reflect the chip name and thus be less than perfect.

- are there some suitable kernel helpers for dealing with big/little
   endianess in lm75_i3c_regmap_bus? Note: I also tried to use the
   non-*_reg callbacks for the regmap bus. But the constified
   void-pointers make it ugly as we need to change buffers because some
   registers are big endian and some little endian.


i3c doesn't seem to have any access functions (kernel helpers) similar to i2c,
other than i3c_device_do_priv_xfers(), so unless those are made available
I think we'll have to bite the bullet and use local access functions.

The other patches look good to me. If you send me a Reviewed-by: and/or
Tested-by: to my patch, I'll queue it all up (except for this patch)
for 6.14.

Thanks,
Guenter





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux