Re: [PATCH v7 5/5] iio: magnetometer: Add mmc5633 sensor

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Oct 28, 2025 at 11:31:11AM -0400, Frank Li wrote:
> On Tue, Oct 28, 2025 at 12:50:55PM +0200, Andy Shevchenko wrote:
> > On Mon, Oct 27, 2025 at 04:08:33PM -0400, Frank Li wrote:

...

> > + time.h // for time constants

First of all, please, please remove the context you are agree with
(non-replying to).

...

> > > +struct mmc5633_data {
> > > +	struct device *dev;
> > > +	struct i3c_device *i3cdev;
> > > +	struct mutex mutex; /* protect to finish one whole measurement */
> > > +	struct regmap *regmap;
> >
> > regmap has struct device, i3c_device presumable also, and here is struct
> > device. Don't we have some overhead?
> 
> i3cdev is used for check it is i2c host or i3c host. If device connect to
> i2c host, i3cdev will be NULL.
> 
> Only if connect to i3c host, driver can use i3c transfer api. The HDR
> command is quite difference with SDR or I2C, which hard to wrap into regmap.
> 
> Anyway we need varible to indicate i3c or i2c. struct i3c_device *i3cdev
> will be simple and needn't force convert struct device in regmap.

This answers only to part of my question. Okay, let's leave it there, why do we
need an explicit struct device then?

> > > +};

...

> > > +static int mmc5633_suspend(struct device *dev)
> > > +{
> > > +	struct mmc5633_data *data = iio_priv(dev_get_drvdata(dev));
> >
> > Than regmap will be derived directly from a device.
> 
> I have not got your idea. Can you point me a example?

dev_get_regmap()

> > > +	regcache_cache_only(data->regmap, true);
> > > +
> > > +	return 0;
> > > +}

...

> > > +	return mmc5633_common_probe(dev, regmap, "mmc5633_i3c", i3cdev);
> >
> > struct i3c_device doesn't have a name, does it?

No answer?


-- 
With Best Regards,
Andy Shevchenko