On Tue, Apr 28, 2020 at 11:33 PM Lee Jones <lee.jones@xxxxxxxxxx> wrote: > > On Tue, 28 Apr 2020, Tim Harvey wrote: > > > On Tue, Apr 28, 2020 at 2:44 AM Lee Jones <lee.jones@xxxxxxxxxx> wrote: > > > > > <snip> > > > > + > > > > +static int gsc_probe(struct i2c_client *client) > > > > +{ > > > > + struct device *dev = &client->dev; > > > > + struct gsc_dev *gsc; > > > > + int ret; > > > > + unsigned int reg; > > > > + > > > > + gsc = devm_kzalloc(dev, sizeof(*gsc), GFP_KERNEL); > > > > + if (!gsc) > > > > + return -ENOMEM; > > > > + > > > > + gsc->dev = &client->dev; > > > > + gsc->i2c = client; > > > > + i2c_set_clientdata(client, gsc); > > > > + > > > > + gsc->bus.reg_write = gsc_regmap_regwrite; > > > > + gsc->bus.reg_read = gsc_regmap_regread; > > > > > > Why do you need to store these in ddata? > > > > Lee, > > > > Thanks for the review! > > > > I need the remap_bus* for devm_regmap_init() in the hwmon sub-module driver: > > > > hwmon->regmap = devm_regmap_init(dev, &gsc->bus, gsc->i2c_hwmon, > > &gsc_hwmon_regmap_config); > > > > Is there something easier I'm missing? > > This is an odd setup. I haven't seen one driver registering another > driver's Regmap call-backs before, related or otherwise. Normally the > Regmap is setup (initialised) in the parent driver and child drivers > just make use of it. Here it looks like you are registering 2 > separate Regmaps, but using the same call-backs for both, which seems > wrong to me. > Lee, It is perhaps an odd setup. The hwmon sub-device is at a different i2c slave address than the other sub-devices. The same callbacks are used for reg read/write to take advantage of the retries due to the errata resulting in occasional NAK'd register reads. Tim