On 17 September 2019 11:56, Marco Felsch wrote: > > > > > This driver is used to cover DA9061, DA9062 and DA9063. This binding > > > therefore > > > > > can apply to any of those as there's no checking of which device is being > used. > > > > > For DA9063 usage, if this option is present then the probe will fail as your > > > > > regmap range update below only takes care of DA9061/2. > > > > > > > > That's right and I only updated the da9061/2 bindings.. > > > > > > D'oh! That's what comes from taking a holiday the week before. :| > > > > Actually I was right the first time. There's one binding covering this driver > > for the 3 devices so my original point was valid although if that register is > > in the valid regmap_range for DA9063 then it would succeed. > > You're right, there is a bit of mixing the naming.. The driver is called > da9063 and the binding is called da9062-onkey.txt.. Anyway, as you said > the regmap_range will be valid for both cases :) Yes, that is a bit misleading. Really the binding document name should really match the driver name. > > > > > > > > > > > > > Ideally I think you should update the da906x_chip_config structure for this > to > > > > > support all devices as I believe the same or similar options are available for > > > > > DA9063. > > > > > > > > After checking the da9062/3 register.h this bitfield is the same for > > > > da9062/3 devices. What about adding a comment here? The CONFIG_I > > > > register is already writeable for the da9063 devices. > > > > > > Given the current implementation of this driver only uses tables to access the > > > correct registers and masks for each device, it would be neater to follow this > > > approach, although I am aware the register addresses and bit fields are the > same. > > That's right because they are needed on other places during the value > evaluation. This register is only set once during probe and shouldn't be > changed afterwards. I just think that given we already have a mechanism to differentiate between the devices for register map access we should probably stick with it. To me it feels a little messy using the direct DA9062 register definition for both devices, but will leave the final word to Dimitry.
