| Message ID | cover.1701971344.git.marcelo.schmitt1@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for AD7091R-2/-4/-8 | expand |
On Thu, 2023-12-07 at 15:37 -0300, Marcelo Schmitt wrote: > Checkpatch presumes attributes marked with __aligned(alignment) are part > of a function declaration and throws a warning stating that those > compiler attributes should have an identifier name which is not correct. > Add __aligned compiler attributes to the list of attribute notes > so they don't cause warnings anymore. > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > --- > The patch that would trigger the mentioned checkpatch warning in this series is > patch number 12 (iio: adc: Add support for AD7091R-8). > > scripts/checkpatch.pl | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 25fdb7fda112..e6773ae0ad08 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -512,6 +512,7 @@ our $Attribute = qr{ > __ro_after_init| > __kprobes| > $InitAttribute| > + __aligned| __aligned takes an argument so I think there needs to have something like the use of __alloc_size below this addition __alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\) maybe __aligned\s*\([^\)]*\) though even that would work well with most uses it would not work with things like drivers/crypto/inside-secure/safexcel_hash.c: u8 cache[HASH_CACHE_SIZE] __aligned(sizeof(u32));
On 12/07, Joe Perches wrote: > On Thu, 2023-12-07 at 15:43 -0300, Marcelo Schmitt wrote: > > The driver for AD7091R was added in > > ca693001: iio: adc: Add support for AD7091R5 ADC > > but no MAINTAINERS file entry was added for it since then. > > Add a proper MAINTAINERS file entry for the AD7091R driver. > [] > > diff --git a/MAINTAINERS b/MAINTAINERS > [] > > @@ -1126,6 +1126,18 @@ F: Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130 > > F: Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml > > F: drivers/iio/adc/ad4130.c > > > > +ANALOG DEVICES INC AD7091R DRIVER > > +M: Marcelo Schmitt <marcelo.schmitt@analog.com> > > +L: linux-iio@vger.kernel.org > > +S: Supported > > +W: http://ez.analog.com/community/linux-device-drivers > > +F: Documentation/devicetree/bindings/iio/adc/adi,ad7091r5.yaml > > +F: Documentation/devicetree/bindings/iio/adc/adi,ad7091r8.yaml > > +F: drivers/iio/adc/drivers/iio/adc/ad7091r-base.c > > +F: drivers/iio/adc/drivers/iio/adc/ad7091r-base.h > > +F: drivers/iio/adc/drivers/iio/adc/ad7091r5.c > > +F: drivers/iio/adc/drivers/iio/adc/ad7091r8.c > > Maybe use wildcards? > > F: Documentation/devicetree/bindings/iio/adc/adi,ad7091r* > F: drivers/iio/adc/drivers/iio/adc/ad7091r* > Good idea, will do for v4. Thanks
On 12/07, Joe Perches wrote: > On Thu, 2023-12-07 at 15:37 -0300, Marcelo Schmitt wrote: > > Checkpatch presumes attributes marked with __aligned(alignment) are part > > of a function declaration and throws a warning stating that those > > compiler attributes should have an identifier name which is not correct. > > Add __aligned compiler attributes to the list of attribute notes > > so they don't cause warnings anymore. > > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > > --- > > The patch that would trigger the mentioned checkpatch warning in this series is > > patch number 12 (iio: adc: Add support for AD7091R-8). > > > > scripts/checkpatch.pl | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 25fdb7fda112..e6773ae0ad08 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -512,6 +512,7 @@ our $Attribute = qr{ > > __ro_after_init| > > __kprobes| > > $InitAttribute| > > + __aligned| > > __aligned takes an argument so I think there needs > to have something like the use of __alloc_size below > this addition > __alloc_size\s*\(\s*\d+\s*(?:,\s*\d+\s*)?\) > > maybe > > __aligned\s*\([^\)]*\) > > though even that would work well with most uses it > would not work with things like > > drivers/crypto/inside-secure/safexcel_hash.c: u8 cache[HASH_CACHE_SIZE] __aligned(sizeof(u32)); > Will think of something that may work for all cases and include in v4. Thanks
On 12/07, David Lechner wrote: > On Thu, Dec 7, 2023 at 12:36 PM Marcelo Schmitt > <marcelo.schmitt@analog.com> wrote: > > > > From: Marcelo Schmitt <marcelo.schmitt1@gmail.com> > > [...] > > > > I see regmap's interface for reading device registers under /sys/kernel/debug/regmap/. > > I can read all registers but can't write to any of them unless I force define > > REGMAP_ALLOW_WRITE_DEBUGFS. > > > > When testing events for this driver I often write to device registers > > to set different rising/falling thresholds. I do something like this: > > # echo 0x17 0x100 > /sys/kernel/debug/iio/iio:device0/direct_reg_access > > > > I tried read/writing to files under iio:device events directory but always > > get segmentation fault. I must be forgetting to implement something. > > What am I missing? > > > > It looks like event callbacks (.read_event_value and friends) are > missing from `static const struct iio_info ad7091r_info = { ... }`. > These callbacks aren't checked for NULL, e.g. in iio_ev_value_show(), > so that is likely where the segfault is happening. Hi David, thank you for pointing that out. Will implement those calls in v4.
From: Marcelo Schmitt <marcelo.schmitt1@gmail.com> ----------------- Updates ----------------- Applied all changes suggested to the previous series. I tried to better explain the changes but, since there is a fair amount of rework in ad7091-base and ad7091r5, it may be hard to get the reasoning for the early patches before looking at the patch for ad7091r8. Change log v2 -> v3: - Split alert fix patch into 2 fix patches and one alignment cleanup patch - Corrected Fixes tag format - Moved MAINTAINERS update to the end of the series - Reworded some commit messages to provide context and make their goal clearer - Removed erroneous gmail sign off - Created container struct to store chip_info, regmap_config, and callbacks specific to each ADC design - Created callbacks for chip specific tasks such as setting device operation mode - Dropped the chip type enum struct - Applied suggestions related to device tree documentation - Added __aligned to list the of checkpatch attribute notes - Other code style tidy ups. I see regmap's interface for reading device registers under /sys/kernel/debug/regmap/. I can read all registers but can't write to any of them unless I force define REGMAP_ALLOW_WRITE_DEBUGFS. When testing events for this driver I often write to device registers to set different rising/falling thresholds. I do something like this: # echo 0x17 0x100 > /sys/kernel/debug/iio/iio:device0/direct_reg_access I tried read/writing to files under iio:device events directory but always get segmentation fault. I must be forgetting to implement something. What am I missing? Thanks Marcelo ----------------- Context ----------------- This series adds support for AD7091R-2/-4/-8 ADCs which can do single shot or sequenced readings. Threshold events are also supported. Overall, AD7091R-2/-4/-8 are very similar to AD7091R-5 except they use SPI interface. Changes have been tested with raspberrypi and eval board on raspberrypi kernel 6.7-rc3 from raspberrypi fork. Link: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad7091r8 Marcelo Schmitt (13): scripts: checkpatch: Add __aligned to the list of attribute notes iio: adc: ad7091r: Populate device driver data field iio: adc: ad7091r: Set alert bit in config register iio: adc: ad7091r: Align arguments to function call parenthesis iio: adc: ad7091r: Move generic AD7091R code to base driver and header file iio: adc: ad7091r: Move chip init data to container struct iio: adc: ad7091r: Set device mode through chip_info callback iio: adc: ad7091r: Enable internal vref if external vref is not supplied iio: adc: ad7091r: Add chip_info callback to get conversion result channel dt-bindings: iio: Add AD7091R-8 iio: adc: Split AD7091R-5 config symbol iio: adc: Add support for AD7091R-8 MAINTAINERS: Add MAINTAINERS entry for AD7091R .../bindings/iio/adc/adi,ad7091r8.yaml | 99 +++++++ MAINTAINERS | 12 + drivers/iio/adc/Kconfig | 16 ++ drivers/iio/adc/Makefile | 4 +- drivers/iio/adc/ad7091r-base.c | 141 ++++------ drivers/iio/adc/ad7091r-base.h | 78 +++++- drivers/iio/adc/ad7091r5.c | 119 ++++---- drivers/iio/adc/ad7091r8.c | 261 ++++++++++++++++++ scripts/checkpatch.pl | 1 + 9 files changed, 597 insertions(+), 134 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7091r8.yaml create mode 100644 drivers/iio/adc/ad7091r8.c