On Sun, Jan 30, 2022 at 6:31 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Sat, 29 Jan 2022 10:24:16 +0530 > jagath jogj <jagathjog1996@xxxxxxxxx> wrote: > > > Hello Jonathan and Andy Shevchenko, > > > > Thanks for replying. > > > > On Fri, Jan 28, 2022 at 8:14 PM Andy Shevchenko > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > > > On Fri, Jan 28, 2022 at 10:35:54AM +0000, Jonathan Cameron wrote: > > > > On Fri, 28 Jan 2022 09:11:28 +0530 > > > > jagath jogj <jagathjog1996@xxxxxxxxx> wrote: > > > > > > > > > Hello, > > > > > > > > > > I have a Maxim DS3502 potentiometer breakout and I have written an IIO > > > > > driver for learning purposes and tested with Raspberry pi and wanted > > > > > to send patches of the driver for the IIO sub-system. > > > > > > > > > > Can I send the patches for DS3502 POT for review? > > > > > > > > > > The setup used to write driver > > > > > Raspberry pi 3b > > > > > DS3502 breakout board > > > > > Raspberry pi latest kernel branch - https://github.com/raspberrypi/linux > > > > > > > Welcome to IIO. > > > > > > > > Absolutely on sending the patches for review. > > > > You'll need to rebase them on latest mainline from kernel.org > > > > (pick a tagged version which would currently be 5.17-rc1_ > > > > I am using raspberry pi kernel branch rpi-5.17-y which is based on > > mainline tag 5.17-rc1. > > Is it required to rebase the changes to the latest tag version > > 5.17-rc1 from kernel.org? > > I'll probably be fine as I wouldn't expect the raspberry pi tree to > be carrying any changes in this area. If there are minor issues I can > usually just fix them up whilst applying. Thank you > > > > > > > > > > > and then follow the documentation for how to submit a patch in > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > > > Sure I will follow the documentation for submitting a patch. > > I am also learning and recently submitted a patch series of code-style > > fixes to the staging branch. > > > > > > > > > > Feel free to ask if you have any questions about the process. > > > > > > > > Looking forwards to seeing your code. > > > > > > Agree with Jonathan. > > > > > > One remark though, can you double check that drivers/iio/potentiometer > > > doesn't have any similar driver that can be expanded (usually it can be > > > done by analyzing a register file of the devices, like register offsets > > > and their meanings or bit fields)? > > Excellent question. > > > > > In iio/potentiometer folder the existing Maxim DS1803 is having some > > differences with DS3502 like > > > > Maxim DS1803: > > Number of wipers - 2 > > Number of Positions - 256 - 8 bit. > > Memory map having 2 volatile registers used to store wiper value. > > https://datasheets.maximintegrated.com/en/ds/DS1803.pdf > > > > > > Maxim DS3502: > > Number of wipers - 1 > > Number of Positions - 128 - 7 bit. > > The memory map has 2 registers to store wiper value and mode > > Supports non-volatile memory to store wiper value > > Supports 2 modes - Mode 0 and Mode 1 > > https://datasheets.maximintegrated.com/en/ds/DS3502.pdf > > > > > > So thought of writing the driver for DS3502 in a separate file. > > Need some advice on this whether to implement it on a separate file or > > to extend the existing driver. > > These potentiometer drivers tend to be very simple, (the ds1803 is > only 167 lines long so you may find that you'd end up adding more > code to make it flexible enough to take your new part than a > new driver would need. > > So perhaps the question we should ask is if we are likely to see > support for a wide range of similar parts? If we are then now > is a good time to make the driver more flexible. > > Working that out will require some datasheet diving.. > My guess is the ds3501 is pretty similar but with some extra bits. Yeah DS3501 is similar to DS3502 with an additional temperature sensor. > > Given these are fairly simple, your best route to an answer might > be to try adding it to the existing driver and see if you run > into any significant complexity. > > It's not unheard of for us to merge drivers together in the future > once it becomes clear that we are supporting lots of similar ones > but it is easier done at the start! > > Jonathan Thanks for your input. I will try to add the DS3502 with the existing driver DS1803 then I will try to add DS3501 to the same driver. There are drivers like iio/temperature/maxim_thermocouple.c and iio/adc/max1027.c where multiple devices with some differences are handled with a single driver file. I will consider them as references to add the ds3502 into existing ds1803.c. > > > > > > > > > > -- > > > With Best Regards, > > > Andy Shevchenko > > > > > > > Regards, > > Jagath > Regards, Jagath
