Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
...
>
> Thanks for answering Jonathan and Stefan.
>
> I would like to contribute with the ad7150 driver. Could you please
> give me some hints on what can I do to move this driver
> out of staging?
>
> I note that device registration (devm_iio_device_register) is not
> the last function called at probe function. Should I change order
> to call dev_info before the device registration?
No on this one. Dev_info is just a message print, so there is nothing
to unwind, hence order doesn't matter. Also in this case it is saying
that the probe really succeeded and as devm_iio_device_register
can fail, it should be after that. However, there is a pretty strong
argument that the print doesn't add anything and should be dropped
anyway.
>
> Also did not see any device tree documentation at
> Documentation/devicetree/bindings/iio for AD7150 driver.
Agreed. That needs writing.
As for the driver:
> /*
> * AD7150 capacitive sensor driver supporting AD7150/1/6
> *
> * Copyright 2010-2011 Analog Devices Inc.
> *
> * Licensed under the GPL-2 or later.
Convert to SPDX. Note that it's inconsistent between here and below
though. See recent clarifying patches from Analog and go with what
they said there. Make sure you include a reference to that thread
in the patch as legal minefields if it isn't clear what your justification
is.
> */
>
> #include
> #include
> #include
> #include
> #include
> #include
>
> #include
> #include
> #include
> /*
> * AD7150 registers definition
> */
>
> #define AD7150_STATUS 0
It's not clear what is a register address and what a field entry here.
There are various ways of doing that. One I like would be
#define AD7150_STATUS_REG0
#define AD7150_STATUS_OUT1 BIT(3)
etc. So use REG to make the register addresses clear
and then use indentation to highlight what is a field name.
> #define AD7150_STATUS_OUT1 BIT(3)
> #define AD7150_STATUS_OUT2 BIT(5)
> #define AD7150_CH1_DATA_HIGH 1
> #define AD7150_CH2_DATA_HIGH 3
> #define AD7150_CH1_AVG_HIGH5
> #define AD7150_CH2_AVG_HIGH7
> #define AD7150_CH1_SENSITIVITY 9
> #define AD7150_CH1_THR_HOLD_H 9
> #define AD7150_CH1_TIMEOUT 10
> #define AD7150_CH1_SETUP 11
> #define AD7150_CH2_SENSITIVITY 12
> #define AD7150_CH2_THR_HOLD_H 12
> #define AD7150_CH2_TIMEOUT 13
> #define AD7150_CH2_SETUP 14
> #define AD7150_CFG 15
> #define AD7150_CFG_FIX BIT(7)
> #define AD7150_PD_TIMER16
> #define AD7150_CH1_CAPDAC 17
> #define AD7150_CH2_CAPDAC 18
> #define AD7150_SN3 19
> #define AD7150_SN2 20
> #define AD7150_SN1 21
> #define AD7150_SN0 22
> #define AD7150_ID 23
>
> /**
> * struct ad7150_chip_info - instance specific chip data
> * @client: i2c client for this device
> * @current_event: device always has one type of event enabled.
> *This element stores the event code of the current one.
> * @threshold: thresholds for simple capacitance value events
> * @thresh_sensitivity: threshold for simple capacitance offset
> *from 'average' value.
> * @mag_sensitity: threshold for magnitude of capacitance offset from
> *from 'average' value.
> * @thresh_timeout: a timeout, in samples from the moment an
> *adaptive threshold event occurs to when the average
> *value jumps to current value.
> * @mag_timeout: a timeout, in sample from the moment an
> *adaptive magnitude event occurs to when the average
> *value jumps to the current value.
> * @old_state: store state from previous event, allowing confirmation
> *of new condition.
> * @conversion_mode: the current conversion mode.
> * @state_lock: ensure consistent state of this structure wrt the
> *hardware.
> */
> struct ad7150_chip_info {
> struct i2c_client *client;
> u64 current_event;
> u16 threshold[2][2];
> u8 thresh_sensitivity[2][2];
> u8 mag_sensitivity[2][2];
> u8 thresh_timeout[2][2];
> u8 mag_timeout[2][2];
> int old_state;
> char *conversion_mode;
> struct mutex state_lock;
> };
>
> /*
> * sysfs nodes
It is both a single line comment and inaccurate. Have a general
clean up of comments to remove those that don't add anything.
> */
>
> static const u8 ad7150_addresses[][6] = {
> { AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> { AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> };
>
> static int ad7150_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val,
> int *val2,
>
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
Em seg, 18 de fev de 2019 às 11:46, Jonathan Cameron escreveu: > > On Thu, 14 Feb 2019 10:51:49 + > "Popa, Stefan Serban" wrote: > > > On Mi, 2019-02-13 at 22:25 -0200, Rodrigo Ribeiro wrote: > > > [External] > > > > > > > > > Em ter, 29 de jan de 2019 às 07:10, Alexandru Ardelean > > l.com> escreveu: > > > > > > > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron > > > wrote: > > > > > > > > > > On Fri, 25 Jan 2019 22:14:32 -0200 > > > > > Rodrigo Ribeiro wrote: > > > > > > > > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > > > > > escreveu: > > > > > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > > > escreveu: > > > > > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > ail.com> wrote: > > > > > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > > > > > Hey, > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > Thanks for answering. > > > > > > > > > > > > > > > A bit curios about this one. > > > > > > > > Are you using this chip/driver ? > > > > > > > > > > > > > > No, I am just a student at USP (University of São Paulo) starting > > > in Linux > > > > > > > Kernel and a new member of the USP Linux Kernel group that has > > > contributed > > > > > > > for a few months. > > > > > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > > > be > > > > > > > > marked for removal (since it's still in staging). > > > > > > > > But if there are users for this driver, it could be left around > > > for a while. > > > > > > > > > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > > removed, I > > > > > > > can send a patch for another driver. Is there any driver that I > > > can > > > > > > > fix a style warning? > > > > > > > > > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I > > > truly > > > > > > contribute to? > > > > > > > > > > How about the ad7150? That one is still listed as production. > > > > > What do you think Alex, you probably have better visibility on the > > > > > status of these parts and their drivers than I do! > > > > > > > > > > (note I haven't even opened that one for a few years so no idea > > > > > what state the driver is in!) > > > > > > > > > > > > > ad7150 is a good alternative. > > > > At a first glance over the driver it looks like it could do with some > > > > polish/conversions to newer IIO constructs (like IIO triggers, maybe > > > > some newer event handling mechanisms?). > > > > I'll sync with Stefan [Popa] to see about this stuff at a later point > > > in time. > > > > > > > > I'd also add here the adxl345 driver. > > > > This is mostly informational for anyone who'd find this interesting. > > > > There are 2 drivers for this chip, one in IIO > > > > [drivers/iio/accel/adxl345] and another one in > > > > "drivers/misc/adxl34x.c" as part of the input sub-system. > > > > What would be interesting here is to finalize the IIO driver [ I think > > > > some features are lacking behind the input driver], and make the input > > > > driver a consumer of the IIO driver. > > > > > > > > From my side, both these variants are fine to take on. > > > > The ad7150 is a good idea as a starter project, and the adxl one is > > > > more of a long-term medium-level project. > > > > > > > > Thanks > > > > Alex > > > > > > > > > > Hi Alex, thanks for suggestions. > > > > > > I read the IIO trigger documentation > > > (https://www.kernel.org/doc/html/v4.12/driver-api/iio/triggers.html) and > > > ask one question: What is the difference between events and triggers? > > > They are sounds me same things. > > > > > > I am trying to understand how to implement a IIO trigger by reading > > > the IIO Simple Dummy code but this driver does not implements IIO > > > triggers > > > but only IIO events. Is there a didactic example like IIO Simple Dummy > > > that implements IIO triggers? > > > > > > Thanks > > > Rodrigo > > > > > > > Hi Rodrigo, > > > > From what I know, IIO Events are not used for passing readings from devices > > to userspace, but rather out of bounds information such as crossing of > > voltage thresholds, proximity detection, motion detection, etc. > > Triggers are typically used to determine when valid data can be read from a > > device which is then stored in the buffer. > > > > After a quick look over the AD7150, I think that using IIO events, might be > > the correct approach, since it is a proximity sensing device. > > To answer the question on generic triggers (agreed, probably not relevant here > from what Stefan has said), there are several software triggers under > drivers/iio/triggers. > > Wasn't a lot of point in adding another one to the dummy driver given you > can use the hrtimer, or sysfs triggers directly. > (loop will result in craziness given the near zero read time of the > dumm
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Thu, 14 Feb 2019 10:51:49 + "Popa, Stefan Serban" wrote: > On Mi, 2019-02-13 at 22:25 -0200, Rodrigo Ribeiro wrote: > > [External] > > > > > > Em ter, 29 de jan de 2019 às 07:10, Alexandru Ardelean > l.com> escreveu: > > > > > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron > > wrote: > > > > > > > > On Fri, 25 Jan 2019 22:14:32 -0200 > > > > Rodrigo Ribeiro wrote: > > > > > > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > > > > escreveu: > > > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > > escreveu: > > > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > ail.com> wrote: > > > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > > > Hey, > > > > > > > > > > > > Hi, > > > > > > > > > > > > Thanks for answering. > > > > > > > > > > > > > A bit curios about this one. > > > > > > > Are you using this chip/driver ? > > > > > > > > > > > > No, I am just a student at USP (University of São Paulo) starting > > in Linux > > > > > > Kernel and a new member of the USP Linux Kernel group that has > > contributed > > > > > > for a few months. > > > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > > be > > > > > > > marked for removal (since it's still in staging). > > > > > > > But if there are users for this driver, it could be left around > > for a while. > > > > > > > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > removed, I > > > > > > can send a patch for another driver. Is there any driver that I > > can > > > > > > fix a style warning? > > > > > > > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I > > truly > > > > > contribute to? > > > > > > > > How about the ad7150? That one is still listed as production. > > > > What do you think Alex, you probably have better visibility on the > > > > status of these parts and their drivers than I do! > > > > > > > > (note I haven't even opened that one for a few years so no idea > > > > what state the driver is in!) > > > > > > > > > > ad7150 is a good alternative. > > > At a first glance over the driver it looks like it could do with some > > > polish/conversions to newer IIO constructs (like IIO triggers, maybe > > > some newer event handling mechanisms?). > > > I'll sync with Stefan [Popa] to see about this stuff at a later point > > in time. > > > > > > I'd also add here the adxl345 driver. > > > This is mostly informational for anyone who'd find this interesting. > > > There are 2 drivers for this chip, one in IIO > > > [drivers/iio/accel/adxl345] and another one in > > > "drivers/misc/adxl34x.c" as part of the input sub-system. > > > What would be interesting here is to finalize the IIO driver [ I think > > > some features are lacking behind the input driver], and make the input > > > driver a consumer of the IIO driver. > > > > > > From my side, both these variants are fine to take on. > > > The ad7150 is a good idea as a starter project, and the adxl one is > > > more of a long-term medium-level project. > > > > > > Thanks > > > Alex > > > > > > > Hi Alex, thanks for suggestions. > > > > I read the IIO trigger documentation > > (https://www.kernel.org/doc/html/v4.12/driver-api/iio/triggers.html) and > > ask one question: What is the difference between events and triggers? > > They are sounds me same things. > > > > I am trying to understand how to implement a IIO trigger by reading > > the IIO Simple Dummy code but this driver does not implements IIO > > triggers > > but only IIO events. Is there a didactic example like IIO Simple Dummy > > that implements IIO triggers? > > > > Thanks > > Rodrigo > > > > Hi Rodrigo, > > From what I know, IIO Events are not used for passing readings from devices > to userspace, but rather out of bounds information such as crossing of > voltage thresholds, proximity detection, motion detection, etc. > Triggers are typically used to determine when valid data can be read from a > device which is then stored in the buffer. > > After a quick look over the AD7150, I think that using IIO events, might be > the correct approach, since it is a proximity sensing device. To answer the question on generic triggers (agreed, probably not relevant here from what Stefan has said), there are several software triggers under drivers/iio/triggers. Wasn't a lot of point in adding another one to the dummy driver given you can use the hrtimer, or sysfs triggers directly. (loop will result in craziness given the near zero read time of the dummy driver ;) Jonathan > > -Stefan > > > > > Jonathan > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > > escreveu: > > > > > > > > > > > > > > O
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Mi, 2019-02-13 at 22:25 -0200, Rodrigo Ribeiro wrote: > [External] > > > Em ter, 29 de jan de 2019 às 07:10, Alexandru Ardelean l.com> escreveu: > > > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron > wrote: > > > > > > On Fri, 25 Jan 2019 22:14:32 -0200 > > > Rodrigo Ribeiro wrote: > > > > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > > > escreveu: > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > escreveu: > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro ail.com> wrote: > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > Hey, > > > > > > > > > > Hi, > > > > > > > > > > Thanks for answering. > > > > > > > > > > > A bit curios about this one. > > > > > > Are you using this chip/driver ? > > > > > > > > > > No, I am just a student at USP (University of São Paulo) starting > in Linux > > > > > Kernel and a new member of the USP Linux Kernel group that has > contributed > > > > > for a few months. > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > be > > > > > > marked for removal (since it's still in staging). > > > > > > But if there are users for this driver, it could be left around > for a while. > > > > > > > > > > This is my first patch in Linux Kernel, but if the driver will be > removed, I > > > > > can send a patch for another driver. Is there any driver that I > can > > > > > fix a style warning? > > > > > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I > truly > > > > contribute to? > > > > > > How about the ad7150? That one is still listed as production. > > > What do you think Alex, you probably have better visibility on the > > > status of these parts and their drivers than I do! > > > > > > (note I haven't even opened that one for a few years so no idea > > > what state the driver is in!) > > > > > > > ad7150 is a good alternative. > > At a first glance over the driver it looks like it could do with some > > polish/conversions to newer IIO constructs (like IIO triggers, maybe > > some newer event handling mechanisms?). > > I'll sync with Stefan [Popa] to see about this stuff at a later point > in time. > > > > I'd also add here the adxl345 driver. > > This is mostly informational for anyone who'd find this interesting. > > There are 2 drivers for this chip, one in IIO > > [drivers/iio/accel/adxl345] and another one in > > "drivers/misc/adxl34x.c" as part of the input sub-system. > > What would be interesting here is to finalize the IIO driver [ I think > > some features are lacking behind the input driver], and make the input > > driver a consumer of the IIO driver. > > > > From my side, both these variants are fine to take on. > > The ad7150 is a good idea as a starter project, and the adxl one is > > more of a long-term medium-level project. > > > > Thanks > > Alex > > > > Hi Alex, thanks for suggestions. > > I read the IIO trigger documentation > (https://www.kernel.org/doc/html/v4.12/driver-api/iio/triggers.html) and > ask one question: What is the difference between events and triggers? > They are sounds me same things. > > I am trying to understand how to implement a IIO trigger by reading > the IIO Simple Dummy code but this driver does not implements IIO > triggers > but only IIO events. Is there a didactic example like IIO Simple Dummy > that implements IIO triggers? > > Thanks > Rodrigo > Hi Rodrigo, From what I know, IIO Events are not used for passing readings from devices to userspace, but rather out of bounds information such as crossing of voltage thresholds, proximity detection, motion detection, etc. Triggers are typically used to determine when valid data can be read from a device which is then stored in the buffer. After a quick look over the AD7150, I think that using IIO events, might be the correct approach, since it is a proximity sensing device. -Stefan > > > Jonathan > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > > > escreveu: > > > > > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro ail.com> wrote: > > > > > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > > > > > Hey, > > > > > > > > > > > > A bit curios about this one. > > > > > > Are you using this chip/driver ? > > > > > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to > be > > > > > > marked for removal (since it's still in staging). > > > > > > But if there are users for this driver, it could be left around > for a while. > > > > > > > > > > > > Thanks > > > > > > Alex > > > > > > > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > > > Signed-off-by: Rafael Tsuha > > > > > > > --- > > > > > > > This macr
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 22:14:32 -0200 > Rodrigo Ribeiro wrote: > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > escreveu: > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > Hi, > > > > > > Thanks for answering. > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > No, I am just a student at USP (University of São Paulo) starting in Linux > > > Kernel and a new member of the USP Linux Kernel group that has contributed > > > for a few months. > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > > removed, I > > > can send a patch for another driver. Is there any driver that I can > > > fix a style warning? > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > > contribute to? > > How about the ad7150? That one is still listed as production. > What do you think Alex, you probably have better visibility on the > status of these parts and their drivers than I do! > > (note I haven't even opened that one for a few years so no idea > what state the driver is in!) > ad7150 is a good alternative. At a first glance over the driver it looks like it could do with some polish/conversions to newer IIO constructs (like IIO triggers, maybe some newer event handling mechanisms?). I'll sync with Stefan [Popa] to see about this stuff at a later point in time. I'd also add here the adxl345 driver. This is mostly informational for anyone who'd find this interesting. There are 2 drivers for this chip, one in IIO [drivers/iio/accel/adxl345] and another one in "drivers/misc/adxl34x.c" as part of the input sub-system. What would be interesting here is to finalize the IIO driver [ I think some features are lacking behind the input driver], and make the input driver a consumer of the IIO driver. >From my side, both these variants are fine to take on. The ad7150 is a good idea as a starter project, and the adxl one is more of a long-term medium-level project. Thanks Alex > Jonathan > > > > > > Thanks > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > > > Thanks > > > > Alex > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > Signed-off-by: Rafael Tsuha > > > > > --- > > > > > This macro is not used anywhere. Should we just correct the > > > > > spelling or remove the macro? > > > > > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > > > b/drivers/staging/iio/cdc/ad7152.c > > > > > index 25f51db..c9da6d4 100644 > > > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > > > @@ -35,7 +35,7 @@ > > > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > > > #define AD7152_REG_CH2_SETUP 14 > > > > > #define AD7152_REG_CFG 15 > > > > > -#define AD7152_REG_RESEVERD16 > > > > > +#define AD7152_REG_RESERVED16 > > > > > #define AD7152_REG_CAPDAC_POS 17 > > > > > #define AD7152_REG_CAPDAC_NEG 18 > > > > > #define AD7152_REG_CFG226 > > > > > -- > > > > > 2.7.4 > > > > > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:09 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 10:19:54 +0200 > Alexandru Ardelean wrote: > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > wrote: > > > > > > Remove the checkpatch.pl check: > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > Hey, > > > > A bit curios about this one. > > Are you using this chip/driver ? > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > marked for removal (since it's still in staging). > > But if there are users for this driver, it could be left around for a while. > > While it might be going away soon, I'm also not that bothered about > the small amount of churn that a tidy up patch like this causes, > and it might not go away ;) > > However it is rather odd to have a 'reserved' entry for a register. > can't see that providing any useful information. Normally I'm > happy to have complete register sets as a form of documentation > if the author wants to do it that way. This however seems silly. > > Alex, we haven't really gone with marking things as 'going away' > before. I'd suggest we take a guess and remove it if you and the > team an analog don't think it's in use. Happy to get a patch for > that if you want to send one. Of course, Rodrigo could do that > patch to get started if that works for everyone? :) > We'll also start a discussion about this particular driver internally. And maybe a procedure for removing drivers that become obsolete [or come close to it]. We also don't/didn't have one for removing "going away" drivers; I just remembered that we took a look over this one and decided not to invest time into it as it's close to being obsolete. Thanks Alex > Jonathan > > > > Thanks > > Alex > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > Signed-off-by: Rafael Tsuha > > > --- > > > This macro is not used anywhere. Should we just correct the > > > spelling or remove the macro? > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > b/drivers/staging/iio/cdc/ad7152.c > > > index 25f51db..c9da6d4 100644 > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > @@ -35,7 +35,7 @@ > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > #define AD7152_REG_CH2_SETUP 14 > > > #define AD7152_REG_CFG 15 > > > -#define AD7152_REG_RESEVERD16 > > > +#define AD7152_REG_RESERVED16 > > > #define AD7152_REG_CAPDAC_POS 17 > > > #define AD7152_REG_CAPDAC_NEG 18 > > > #define AD7152_REG_CFG226 > > > -- > > > 2.7.4 > > > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Fri, 25 Jan 2019 22:14:32 -0200, Rodrigo Ribeiro said: > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > contribute to? I'll give you a pointer to the "How to contribute" the Kernel Newbies list and IRC channel uses: https://lists.kernelnewbies.org/pipermail/kernelnewbies/2017-April/017765.html
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Fri, 25 Jan 2019 22:14:32 -0200 Rodrigo Ribeiro wrote: > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > escreveu: > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > escreveu: > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > wrote: > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > Hey, > > > > Hi, > > > > Thanks for answering. > > > > > A bit curios about this one. > > > Are you using this chip/driver ? > > > > No, I am just a student at USP (University of São Paulo) starting in Linux > > Kernel and a new member of the USP Linux Kernel group that has contributed > > for a few months. > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > marked for removal (since it's still in staging). > > > But if there are users for this driver, it could be left around for a > > > while. > > > > This is my first patch in Linux Kernel, but if the driver will be removed, I > > can send a patch for another driver. Is there any driver that I can > > fix a style warning? > > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > contribute to? How about the ad7150? That one is still listed as production. What do you think Alex, you probably have better visibility on the status of these parts and their drivers than I do! (note I haven't even opened that one for a few years so no idea what state the driver is in!) Jonathan > > > Thanks > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > escreveu: > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > wrote: > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > Hey, > > > > > > A bit curios about this one. > > > Are you using this chip/driver ? > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > marked for removal (since it's still in staging). > > > But if there are users for this driver, it could be left around for a > > > while. > > > > > > Thanks > > > Alex > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > Signed-off-by: Rafael Tsuha > > > > --- > > > > This macro is not used anywhere. Should we just correct the > > > > spelling or remove the macro? > > > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > > b/drivers/staging/iio/cdc/ad7152.c > > > > index 25f51db..c9da6d4 100644 > > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > > @@ -35,7 +35,7 @@ > > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > > #define AD7152_REG_CH2_SETUP 14 > > > > #define AD7152_REG_CFG 15 > > > > -#define AD7152_REG_RESEVERD16 > > > > +#define AD7152_REG_RESERVED16 > > > > #define AD7152_REG_CAPDAC_POS 17 > > > > #define AD7152_REG_CAPDAC_NEG 18 > > > > #define AD7152_REG_CFG226 > > > > -- > > > > 2.7.4 > > > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Fri, 25 Jan 2019 10:19:54 +0200 Alexandru Ardelean wrote: > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > > > Remove the checkpatch.pl check: > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > Hey, > > A bit curios about this one. > Are you using this chip/driver ? > > Thing is: the part is nearing EOL, and it could be an idea to be > marked for removal (since it's still in staging). > But if there are users for this driver, it could be left around for a while. While it might be going away soon, I'm also not that bothered about the small amount of churn that a tidy up patch like this causes, and it might not go away ;) However it is rather odd to have a 'reserved' entry for a register. can't see that providing any useful information. Normally I'm happy to have complete register sets as a form of documentation if the author wants to do it that way. This however seems silly. Alex, we haven't really gone with marking things as 'going away' before. I'd suggest we take a guess and remove it if you and the team an analog don't think it's in use. Happy to get a patch for that if you want to send one. Of course, Rodrigo could do that patch to get started if that works for everyone? :) Jonathan > > Thanks > Alex > > > > > Signed-off-by: Rodrigo Ribeiro > > Signed-off-by: Rafael Tsuha > > --- > > This macro is not used anywhere. Should we just correct the > > spelling or remove the macro? > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > b/drivers/staging/iio/cdc/ad7152.c > > index 25f51db..c9da6d4 100644 > > --- a/drivers/staging/iio/cdc/ad7152.c > > +++ b/drivers/staging/iio/cdc/ad7152.c > > @@ -35,7 +35,7 @@ > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > #define AD7152_REG_CH2_SETUP 14 > > #define AD7152_REG_CFG 15 > > -#define AD7152_REG_RESEVERD16 > > +#define AD7152_REG_RESERVED16 > > #define AD7152_REG_CAPDAC_POS 17 > > #define AD7152_REG_CAPDAC_NEG 18 > > #define AD7152_REG_CFG226 > > -- > > 2.7.4 > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro escreveu: > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > escreveu: > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > wrote: > > > > > > Remove the checkpatch.pl check: > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > Hey, > > Hi, > > Thanks for answering. > > > A bit curios about this one. > > Are you using this chip/driver ? > > No, I am just a student at USP (University of São Paulo) starting in Linux > Kernel and a new member of the USP Linux Kernel group that has contributed > for a few months. > > > Thing is: the part is nearing EOL, and it could be an idea to be > > marked for removal (since it's still in staging). > > But if there are users for this driver, it could be left around for a while. > > This is my first patch in Linux Kernel, but if the driver will be removed, I > can send a patch for another driver. Is there any driver that I can > fix a style warning? Maybe, one checkstyle patch is enough, right? Which drivers can I truly contribute to? > Thanks > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > escreveu: > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > wrote: > > > > > > Remove the checkpatch.pl check: > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > Hey, > > > > A bit curios about this one. > > Are you using this chip/driver ? > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > marked for removal (since it's still in staging). > > But if there are users for this driver, it could be left around for a while. > > > > Thanks > > Alex > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > Signed-off-by: Rafael Tsuha > > > --- > > > This macro is not used anywhere. Should we just correct the > > > spelling or remove the macro? > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > b/drivers/staging/iio/cdc/ad7152.c > > > index 25f51db..c9da6d4 100644 > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > @@ -35,7 +35,7 @@ > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > #define AD7152_REG_CH2_SETUP 14 > > > #define AD7152_REG_CFG 15 > > > -#define AD7152_REG_RESEVERD16 > > > +#define AD7152_REG_RESERVED16 > > > #define AD7152_REG_CAPDAC_POS 17 > > > #define AD7152_REG_CAPDAC_NEG 18 > > > #define AD7152_REG_CFG226 > > > -- > > > 2.7.4 > > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean escreveu: > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > > > Remove the checkpatch.pl check: > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > Hey, Hi, Thanks for answering. > A bit curios about this one. > Are you using this chip/driver ? No, I am just a student at USP (University of São Paulo) starting in Linux Kernel and a new member of the USP Linux Kernel group that has contributed for a few months. > Thing is: the part is nearing EOL, and it could be an idea to be > marked for removal (since it's still in staging). > But if there are users for this driver, it could be left around for a while. This is my first patch in Linux Kernel, but if the driver will be removed, I can send a patch for another driver. Is there any driver that I can fix a style warning? Thanks Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean escreveu: > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > > > Remove the checkpatch.pl check: > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > Hey, > > A bit curios about this one. > Are you using this chip/driver ? > > Thing is: the part is nearing EOL, and it could be an idea to be > marked for removal (since it's still in staging). > But if there are users for this driver, it could be left around for a while. > > Thanks > Alex > > > > > Signed-off-by: Rodrigo Ribeiro > > Signed-off-by: Rafael Tsuha > > --- > > This macro is not used anywhere. Should we just correct the > > spelling or remove the macro? > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > b/drivers/staging/iio/cdc/ad7152.c > > index 25f51db..c9da6d4 100644 > > --- a/drivers/staging/iio/cdc/ad7152.c > > +++ b/drivers/staging/iio/cdc/ad7152.c > > @@ -35,7 +35,7 @@ > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > #define AD7152_REG_CH2_SETUP 14 > > #define AD7152_REG_CFG 15 > > -#define AD7152_REG_RESEVERD16 > > +#define AD7152_REG_RESERVED16 > > #define AD7152_REG_CAPDAC_POS 17 > > #define AD7152_REG_CAPDAC_NEG 18 > > #define AD7152_REG_CFG226 > > -- > > 2.7.4 > >
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > Remove the checkpatch.pl check: > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? Hey, A bit curios about this one. Are you using this chip/driver ? Thing is: the part is nearing EOL, and it could be an idea to be marked for removal (since it's still in staging). But if there are users for this driver, it could be left around for a while. Thanks Alex > > Signed-off-by: Rodrigo Ribeiro > Signed-off-by: Rafael Tsuha > --- > This macro is not used anywhere. Should we just correct the > spelling or remove the macro? > > drivers/staging/iio/cdc/ad7152.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c > b/drivers/staging/iio/cdc/ad7152.c > index 25f51db..c9da6d4 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -35,7 +35,7 @@ > #define AD7152_REG_CH2_GAIN_HIGH 12 > #define AD7152_REG_CH2_SETUP 14 > #define AD7152_REG_CFG 15 > -#define AD7152_REG_RESEVERD16 > +#define AD7152_REG_RESERVED16 > #define AD7152_REG_CAPDAC_POS 17 > #define AD7152_REG_CAPDAC_NEG 18 > #define AD7152_REG_CFG226 > -- > 2.7.4 >
[PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
Remove the checkpatch.pl check: CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? Signed-off-by: Rodrigo Ribeiro Signed-off-by: Rafael Tsuha --- This macro is not used anywhere. Should we just correct the spelling or remove the macro? drivers/staging/iio/cdc/ad7152.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c index 25f51db..c9da6d4 100644 --- a/drivers/staging/iio/cdc/ad7152.c +++ b/drivers/staging/iio/cdc/ad7152.c @@ -35,7 +35,7 @@ #define AD7152_REG_CH2_GAIN_HIGH 12 #define AD7152_REG_CH2_SETUP 14 #define AD7152_REG_CFG 15 -#define AD7152_REG_RESEVERD16 +#define AD7152_REG_RESERVED16 #define AD7152_REG_CAPDAC_POS 17 #define AD7152_REG_CAPDAC_NEG 18 #define AD7152_REG_CFG226 -- 2.7.4
