lkml.org 
[lkml]   [2023]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
    On Fri, 26 May 2023 16:30:46 +0200
    Mehdi Djait <mehdi.djait.k@gmail.com> wrote:

    > Add the chip_info structure to the driver's private data to hold all
    > the device specific infos.
    > Refactor the kx022a driver implementation to make it more generic and
    > extensible.
    >
    > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>

    Hi Mehdi,

    Just one trivial comment from me about missing docs / unused element.

    Thanks,

    Jonathan

    ...

    > diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
    > index 12424649d438..c23b6f03409e 100644
    > --- a/drivers/iio/accel/kionix-kx022a.h
    > +++ b/drivers/iio/accel/kionix-kx022a.h
    > @@ -76,7 +76,56 @@
    >
    > struct device;
    >
    > -int kx022a_probe_internal(struct device *dev);
    > -extern const struct regmap_config kx022a_regmap;
    > +/**
    > + * struct kx022a_chip_info - Kionix accelerometer chip specific information
    > + *
    > + * @name: name of the device
    > + * @regmap_config: pointer to register map configuration
    > + * @channels: pointer to iio_chan_spec array
    > + * @num_channels: number of iio_chan_spec channels
    > + * @fifo_length: number of 16-bit samples in a full buffer
    > + * @who: WHO_AM_I register
    > + * @id: WHO_AM_I register value
    > + * @cntl: control register 1
    > + * @cntl2: control register 2
    > + * @odcntl: output data control register
    > + * @buf_cntl1: buffer control register 1
    > + * @buf_cntl2: buffer control register 2
    > + * @buf_clear: buffer clear register
    > + * @buf_status1: buffer status register 1

    Missing entry for buf_smp_lvl_mask.

    Mind you it's also not used in this patch and only seems to be used in the
    get_fifo_bytes() function in patch 7, so might not be needed at all?
    If it is, then introduce it in patch 7 where we can see how it is used.

    > + * @buf_read: buffer read register
    > + * @inc1: interrupt control register 1
    > + * @inc4: interrupt control register 4
    > + * @inc5: interrupt control register 5
    > + * @inc6: interrupt control register 6
    > + * @xout_l: x-axis output least significant byte
    > + */
    > +struct kx022a_chip_info {
    > + const char *name;
    > + const struct regmap_config *regmap_config;
    > + const struct iio_chan_spec *channels;
    > + unsigned int num_channels;
    > + unsigned int fifo_length;
    > + u8 who;
    > + u8 id;
    > + u8 cntl;
    > + u8 cntl2;
    > + u8 odcntl;
    > + u8 buf_cntl1;
    > + u8 buf_cntl2;
    > + u8 buf_clear;
    > + u8 buf_status1;
    > + u16 buf_smp_lvl_mask;
    > + u8 buf_read;
    > + u8 inc1;
    > + u8 inc4;
    > + u8 inc5;
    > + u8 inc6;
    > + u8 xout_l;
    > +};
    > +
    > +int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info);
    > +
    > +extern const struct kx022a_chip_info kx022a_chip_info;
    >
    > #endif

    \
     
     \ /
      Last update: 2023-05-28 18:40    [W:5.111 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site