| Message ID | 20200725062938.15388-1-bharadwaj.rohit8@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v4] staging: nvec: change usage of slave to secondary | expand |
On Sat, Jul 25, 2020 at 11:59:39AM +0530, Rohit K Bharadwaj wrote:
> changed usage of slave (which is deprecated) to secondary without breaking the driver
The relevant I2C and SMBus standards use master/slave terminology. Why are
you changing the names to something unfamiliar?
If the reason are the recent coding-style changes, then please note they
are about avoiding introducing *NEW* uses of the specific words and not
about blindly replacing existing occurrences.
Best Regards
Michał Mirosław
On 25/07/20 5:31 pm, Michał Mirosław wrote: > On Sat, Jul 25, 2020 at 11:59:39AM +0530, Rohit K Bharadwaj wrote: >> changed usage of slave (which is deprecated) to secondary without breaking the driver > > The relevant I2C and SMBus standards use master/slave terminology. Why are > you changing the names to something unfamiliar? > > If the reason are the recent coding-style changes, then please note they > are about avoiding introducing *NEW* uses of the specific words and not > about blindly replacing existing occurrences. > > Best Regards > Michał Mirosław > I'm really sorry sir, I didn't knew about this, yes the reason for my change is according to the script checkpatch.pl to suit the coding style, I would try to fix some other coding style related issues if this patch seems to be a bad idea.
On Sat, Jul 25, 2020 at 06:01:19PM +0530, Rohit K Bharadwaj wrote: > On 25/07/20 5:31 pm, Michał Mirosław wrote: > > On Sat, Jul 25, 2020 at 11:59:39AM +0530, Rohit K Bharadwaj wrote: > >> changed usage of slave (which is deprecated) to secondary without breaking the driver > > > > The relevant I2C and SMBus standards use master/slave terminology. Why are > > you changing the names to something unfamiliar? > > > > If the reason are the recent coding-style changes, then please note they > > are about avoiding introducing *NEW* uses of the specific words and not > > about blindly replacing existing occurrences. > > I'm really sorry sir, I didn't knew about this, yes the reason for my change is according to the script checkpatch.pl to suit the coding style, I would try to fix some other coding style related issues if this patch seems to be a bad idea. I didn't know checkpatch does this (it doesn't in current Linus' master tree). I can see there is a commit in next adding this, but seems that it uses a test far from the original coding-style wording... Best Regards Michał Mirosław
On 25/07/20 6:20 pm, Michał Mirosław wrote: > On Sat, Jul 25, 2020 at 06:01:19PM +0530, Rohit K Bharadwaj wrote: >> On 25/07/20 5:31 pm, Michał Mirosław wrote: >>> On Sat, Jul 25, 2020 at 11:59:39AM +0530, Rohit K Bharadwaj wrote: >>>> changed usage of slave (which is deprecated) to secondary without breaking the driver >>> >>> The relevant I2C and SMBus standards use master/slave terminology. Why are >>> you changing the names to something unfamiliar? >>> >>> If the reason are the recent coding-style changes, then please note they >>> are about avoiding introducing *NEW* uses of the specific words and not >>> about blindly replacing existing occurrences. >> >> I'm really sorry sir, I didn't knew about this, yes the reason for my change is according to the script checkpatch.pl to suit the coding style, I would try to fix some other coding style related issues if this patch seems to be a bad idea. > > I didn't know checkpatch does this (it doesn't in current Linus' master > tree). I can see there is a commit in next adding this, but seems that > it uses a test far from the original coding-style wording... > > Best Regards > Michał Mirosław > yes sir, in the linux-next tree, when I ran the script on this file it showed me it had style issues and the usage of slave is deprecated and it suggested me to replace it with secondary or target. and hence I made this patch, please do let me know if this patch can be acceptable or not.
Hi, On Sat, 25 Jul 2020, Michał Mirosław wrote: > On Sat, Jul 25, 2020 at 06:01:19PM +0530, Rohit K Bharadwaj wrote: >> On 25/07/20 5:31 pm, Michał Mirosław wrote: >>> On Sat, Jul 25, 2020 at 11:59:39AM +0530, Rohit K Bharadwaj wrote: >>>> changed usage of slave (which is deprecated) to secondary without breaking the driver >>> >>> The relevant I2C and SMBus standards use master/slave terminology. Why are >>> you changing the names to something unfamiliar? >>> >>> If the reason are the recent coding-style changes, then please note they >>> are about avoiding introducing *NEW* uses of the specific words and not >>> about blindly replacing existing occurrences. >> >> I'm really sorry sir, I didn't knew about this, yes the reason for my change is according to the script checkpatch.pl to suit the coding style, I would try to fix some other coding style related issues if this patch seems to be a bad idea. > > I didn't know checkpatch does this (it doesn't in current Linus' master > tree). I can see there is a commit in next adding this, but seems that > it uses a test far from the original coding-style wording... given the discussion here [1] and also looking at the coding style patch here [2], I think this patch should not be applied. The slave term here comes from the I2C protocol (which we can't change) which is listed as an exception in [2], see below: "+Exceptions for introducing new usage is to maintain a userspace ABI/API, +or when updating code for an existing (as of 2020) hardware or protocol +specification that mandates those terms. For new specifications +translate specification usage of the terminology to the kernel coding +standard where possible. " Marc [1] https://lkml.org/lkml/2020/6/11/60 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/process/coding-style.rst?id=a5f526ec
Hello Rohit, On Sat, 25 Jul 2020, Rohit K Bharadwaj wrote: > changed usage of slave (which is deprecated) to secondary without breaking the driver > > Tested-by: Dan Carpenter <dan.carpenter@oracle.com> > Acked-by: Marc Dietrich <marvin24@posteo.de> > Signed-off-by: Rohit K Bharadwaj <bharadwaj.rohit8@gmail.com> please don't add "*-by"'s by yourself when you send a new patch version. These will be added "automatically" during the patch handling. I just said, I *will* ack your patch, when you resent it, not that I did it already. Thanks! Marc > --- > v4: undo the changes (which broke the driver) to this line: if (of_property_read_u32(dev->of_node, "slave-addr", &nvec->i2c_addr)) > v3: change patch subject, add version history > v2: add changelog text in body of mail > v1: fix style issues by changing usage of slave to secondary > > drivers/staging/nvec/nvec.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c > index 360ec0407740..a7e995bfe989 100644 > --- a/drivers/staging/nvec/nvec.c > +++ b/drivers/staging/nvec/nvec.c > @@ -718,7 +718,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) > return IRQ_HANDLED; > } > > -static void tegra_init_i2c_slave(struct nvec_chip *nvec) > +static void tegra_init_i2c_secondary(struct nvec_chip *nvec) > { > u32 val; > > @@ -744,7 +744,7 @@ static void tegra_init_i2c_slave(struct nvec_chip *nvec) > } > > #ifdef CONFIG_PM_SLEEP > -static void nvec_disable_i2c_slave(struct nvec_chip *nvec) > +static void nvec_disable_i2c_secondary(struct nvec_chip *nvec) > { > disable_irq(nvec->irq); > writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG); > @@ -839,7 +839,7 @@ static int tegra_nvec_probe(struct platform_device *pdev) > } > disable_irq(nvec->irq); > > - tegra_init_i2c_slave(nvec); > + tegra_init_i2c_secondary(nvec); > > /* enable event reporting */ > nvec_toggle_global_events(nvec, true); > @@ -913,7 +913,7 @@ static int nvec_suspend(struct device *dev) > if (!err) > nvec_msg_free(nvec, msg); > > - nvec_disable_i2c_slave(nvec); > + nvec_disable_i2c_secondary(nvec); > > return 0; > } > @@ -923,7 +923,7 @@ static int nvec_resume(struct device *dev) > struct nvec_chip *nvec = dev_get_drvdata(dev); > > dev_dbg(nvec->dev, "resuming\n"); > - tegra_init_i2c_slave(nvec); > + tegra_init_i2c_secondary(nvec); > nvec_toggle_global_events(nvec, true); > > return 0; > -- > 2.25.1 > >
On 02/08/20 1:43 pm, Marc Dietrich wrote: > Hello Rohit, > > On Sat, 25 Jul 2020, Rohit K Bharadwaj wrote: > >> changed usage of slave (which is deprecated) to secondary without breaking the driver >> >> Tested-by: Dan Carpenter <dan.carpenter@oracle.com> >> Acked-by: Marc Dietrich <marvin24@posteo.de> >> Signed-off-by: Rohit K Bharadwaj <bharadwaj.rohit8@gmail.com> > > please don't add "*-by"'s by yourself when you send a new patch version. > These will be added "automatically" during the patch handling. I just said, I *will* ack your patch, when you resent it, not that I did it already. > > Thanks! > > Marc > >> --- >> v4: undo the changes (which broke the driver) to this line: if (of_property_read_u32(dev->of_node, "slave-addr", &nvec->i2c_addr)) >> v3: change patch subject, add version history >> v2: add changelog text in body of mail >> v1: fix style issues by changing usage of slave to secondary >> >> drivers/staging/nvec/nvec.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c >> index 360ec0407740..a7e995bfe989 100644 >> --- a/drivers/staging/nvec/nvec.c >> +++ b/drivers/staging/nvec/nvec.c >> @@ -718,7 +718,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) >> return IRQ_HANDLED; >> } >> >> -static void tegra_init_i2c_slave(struct nvec_chip *nvec) >> +static void tegra_init_i2c_secondary(struct nvec_chip *nvec) >> { >> u32 val; >> >> @@ -744,7 +744,7 @@ static void tegra_init_i2c_slave(struct nvec_chip *nvec) >> } >> >> #ifdef CONFIG_PM_SLEEP >> -static void nvec_disable_i2c_slave(struct nvec_chip *nvec) >> +static void nvec_disable_i2c_secondary(struct nvec_chip *nvec) >> { >> disable_irq(nvec->irq); >> writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG); >> @@ -839,7 +839,7 @@ static int tegra_nvec_probe(struct platform_device *pdev) >> } >> disable_irq(nvec->irq); >> >> - tegra_init_i2c_slave(nvec); >> + tegra_init_i2c_secondary(nvec); >> >> /* enable event reporting */ >> nvec_toggle_global_events(nvec, true); >> @@ -913,7 +913,7 @@ static int nvec_suspend(struct device *dev) >> if (!err) >> nvec_msg_free(nvec, msg); >> >> - nvec_disable_i2c_slave(nvec); >> + nvec_disable_i2c_secondary(nvec); >> >> return 0; >> } >> @@ -923,7 +923,7 @@ static int nvec_resume(struct device *dev) >> struct nvec_chip *nvec = dev_get_drvdata(dev); >> >> dev_dbg(nvec->dev, "resuming\n"); >> - tegra_init_i2c_slave(nvec); >> + tegra_init_i2c_secondary(nvec); >> nvec_toggle_global_events(nvec, true); >> >> return 0; >> -- >> 2.25.1 >> >> I'm sorry for the tags sir, I would make sure not to make the mistakes in future, Thanks for taking your time, I hope I can contribute on other aspects of Linux kernel.
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c index 360ec0407740..a7e995bfe989 100644 --- a/drivers/staging/nvec/nvec.c +++ b/drivers/staging/nvec/nvec.c @@ -718,7 +718,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) return IRQ_HANDLED; } -static void tegra_init_i2c_slave(struct nvec_chip *nvec) +static void tegra_init_i2c_secondary(struct nvec_chip *nvec) { u32 val; @@ -744,7 +744,7 @@ static void tegra_init_i2c_slave(struct nvec_chip *nvec) } #ifdef CONFIG_PM_SLEEP -static void nvec_disable_i2c_slave(struct nvec_chip *nvec) +static void nvec_disable_i2c_secondary(struct nvec_chip *nvec) { disable_irq(nvec->irq); writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG); @@ -839,7 +839,7 @@ static int tegra_nvec_probe(struct platform_device *pdev) } disable_irq(nvec->irq); - tegra_init_i2c_slave(nvec); + tegra_init_i2c_secondary(nvec); /* enable event reporting */ nvec_toggle_global_events(nvec, true); @@ -913,7 +913,7 @@ static int nvec_suspend(struct device *dev) if (!err) nvec_msg_free(nvec, msg); - nvec_disable_i2c_slave(nvec); + nvec_disable_i2c_secondary(nvec); return 0; } @@ -923,7 +923,7 @@ static int nvec_resume(struct device *dev) struct nvec_chip *nvec = dev_get_drvdata(dev); dev_dbg(nvec->dev, "resuming\n"); - tegra_init_i2c_slave(nvec); + tegra_init_i2c_secondary(nvec); nvec_toggle_global_events(nvec, true); return 0;