RE: [PATCH 2/4] input: misc: da9063_onkey: add mode change support

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

 



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.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux