[PATCH] checkpatch: Add exceptions for dsb keyword usage

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[Prakruthi Deepak Heragu]

mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
is discouraged. However, there are exceptions when dsb is used in a
variable or a function name. Exceptions are when 'dsb' is prefixed with
class [-_>*\.] and/or suffixed with class [-_\.;].

Signed-off-by: Prakruthi Deepak Heragu <pheragu@codeaurora.org>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a9c0550..978c752 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5372,6 +5372,12 @@ sub process {
 			     "Avoid line continuations in quoted strings\n" . $herecurr);
 		}
 
+# dsb is too ARMish, and should usually be mb.
+        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
+            WARN("ARM_BARRIER",
+                 "Use of dsb is discouranged: prefer mb.\n" .
+                 $herecurr);
+		}
 # warn about #if 0
 		if ($line =~ /^.\s*\#\s*if\s+0\b/) {
 			CHK("REDUNDANT_CODE",
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[Joe Perches]

On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> is discouraged. However, there are exceptions when dsb is used in a
> variable or a function name. Exceptions are when 'dsb' is prefixed with
> class [-_>*\.] and/or suffixed with class [-_\.;].
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5372,6 +5372,12 @@ sub process {
>  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>  		}
>  
> +# dsb is too ARMish, and should usually be mb.
> +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> +            WARN("ARM_BARRIER",
> +                 "Use of dsb is discouranged: prefer mb.\n" .
> +                 $herecurr);
> +		}

This patch is whitespace damaged with a spelling error.

Also, if this is reasonable test, and I don't know
that it is, it should be cc'd to the linux-arm list
linux-arm-kernel at lists.infradead.org

Also, I suggest 2 tests, one for .S files and
another for .[ch] files, and this be made specific
to arch/arm... files

Something like:

	if ($realfile =~ @^arch/arm@ &&
	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
		WARN("ARM_DSB",
		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
	}

ARM people, is this reasonable?

Re: [PATCH] checkpatch: Add exceptions for dsb keyword usage

[Joe Perches]

On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> is discouraged. However, there are exceptions when dsb is used in a
> variable or a function name. Exceptions are when 'dsb' is prefixed with
> class [-_>*\.] and/or suffixed with class [-_\.;].
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5372,6 +5372,12 @@ sub process {
>  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>  		}
>  
> +# dsb is too ARMish, and should usually be mb.
> +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> +            WARN("ARM_BARRIER",
> +                 "Use of dsb is discouranged: prefer mb.\n" .
> +                 $herecurr);
> +		}

This patch is whitespace damaged with a spelling error.

Also, if this is reasonable test, and I don't know
that it is, it should be cc'd to the linux-arm list
linux-arm-kernel@lists.infradead.org

Also, I suggest 2 tests, one for .S files and
another for .[ch] files, and this be made specific
to arch/arm... files

Something like:

	if ($realfile =~ @^arch/arm@ &&
	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
		WARN("ARM_DSB",
		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
	}

ARM people, is this reasonable?

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[Mark Rutland]

On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
> On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> > is discouraged. However, there are exceptions when dsb is used in a
> > variable or a function name. Exceptions are when 'dsb' is prefixed with
> > class [-_>*\.] and/or suffixed with class [-_\.;].

This is a really confusing way of describing the match behaviour, and doesn't
explain why this is a big problem.

In C it's either:

	dsb()
	dsb(scope)	// e.g. dsb(ish)

... where scope is [a-z]*.

... which can be matched as something like 'dsb([a-z]*)' if necessary.

> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -5372,6 +5372,12 @@ sub process {
> >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
> >  		}
> >  
> > +# dsb is too ARMish, and should usually be mb.
> > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> > +            WARN("ARM_BARRIER",
> > +                 "Use of dsb is discouranged: prefer mb.\n" .
> > +                 $herecurr);
> > +		}
> 
> This patch is whitespace damaged with a spelling error.
> 
> Also, if this is reasonable test, and I don't know
> that it is, it should be cc'd to the linux-arm list
> linux-arm-kernel at lists.infradead.org
> 
> Also, I suggest 2 tests, one for .S files and
> another for .[ch] files, and this be made specific
> to arch/arm... files
> 
> Something like:
> 
> 	if ($realfile =~ @^arch/arm@ &&
> 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
> 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
> 		WARN("ARM_DSB",
> 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
> 	}
> 
> ARM people, is this reasonable?

I don't think this is a big deal today. 

For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.

For code *ouside* of arch/{arm,arm64}, there are a number of cases where we
want to use dsb(), e.g. when dealing with architectural drivers that require
special barriers, or for common code shared across arm and arm64.

It doesn't look like this is a big problem today, anyhow:

[mark at salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch          
drivers/irqchip/irq-armada-370-xp.c:    dsb();
drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/perf/arm_spe_pmu.c:     dsb(nsh);
drivers/perf/arm_spe_pmu.c:     dsb(nsh);
drivers/power/reset/arm-versatile-reboot.c:     dsb();
drivers/soc/rockchip/pm_domains.c:      dsb(sy);
drivers/soc/rockchip/pm_domains.c:      dsb(sy);
drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define dsb(a)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:     dsb(sy);         /* data barrier operation */
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         dsb(sy);
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);

Thanks,
Mark.

Re: [PATCH] checkpatch: Add exceptions for dsb keyword usage

[Mark Rutland]

On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
> On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> > is discouraged. However, there are exceptions when dsb is used in a
> > variable or a function name. Exceptions are when 'dsb' is prefixed with
> > class [-_>*\.] and/or suffixed with class [-_\.;].

This is a really confusing way of describing the match behaviour, and doesn't
explain why this is a big problem.

In C it's either:

	dsb()
	dsb(scope)	// e.g. dsb(ish)

... where scope is [a-z]*.

... which can be matched as something like 'dsb([a-z]*)' if necessary.

> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -5372,6 +5372,12 @@ sub process {
> >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
> >  		}
> >  
> > +# dsb is too ARMish, and should usually be mb.
> > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> > +            WARN("ARM_BARRIER",
> > +                 "Use of dsb is discouranged: prefer mb.\n" .
> > +                 $herecurr);
> > +		}
> 
> This patch is whitespace damaged with a spelling error.
> 
> Also, if this is reasonable test, and I don't know
> that it is, it should be cc'd to the linux-arm list
> linux-arm-kernel@lists.infradead.org
> 
> Also, I suggest 2 tests, one for .S files and
> another for .[ch] files, and this be made specific
> to arch/arm... files
> 
> Something like:
> 
> 	if ($realfile =~ @^arch/arm@ &&
> 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
> 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
> 		WARN("ARM_DSB",
> 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
> 	}
> 
> ARM people, is this reasonable?

I don't think this is a big deal today. 

For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.

For code *ouside* of arch/{arm,arm64}, there are a number of cases where we
want to use dsb(), e.g. when dealing with architectural drivers that require
special barriers, or for common code shared across arm and arm64.

It doesn't look like this is a big problem today, anyhow:

[mark@salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch          
drivers/irqchip/irq-armada-370-xp.c:    dsb();
drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
drivers/perf/arm_spe_pmu.c:     dsb(nsh);
drivers/perf/arm_spe_pmu.c:     dsb(nsh);
drivers/power/reset/arm-versatile-reboot.c:     dsb();
drivers/soc/rockchip/pm_domains.c:      dsb(sy);
drivers/soc/rockchip/pm_domains.c:      dsb(sy);
drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define dsb(a)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:     dsb(sy);         /* data barrier operation */
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         dsb(sy);
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);

Thanks,
Mark.

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[Joe Perches]

On Fri, 2018-07-06 at 06:45 +0100, Mark Rutland wrote:
> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
> > On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> > > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> > > is discouraged. However, there are exceptions when dsb is used in a
> > > variable or a function name. Exceptions are when 'dsb' is prefixed with
> > > class [-_>*\.] and/or suffixed with class [-_\.;].
> 
> This is a really confusing way of describing the match behaviour, and doesn't
> explain why this is a big problem.
> 
> In C it's either:
> 
> 	dsb()
> 	dsb(scope)	// e.g. dsb(ish)
> 
> ... where scope is [a-z]*.
> 
> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
> 
> > []
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > 
> > []
> > > @@ -5372,6 +5372,12 @@ sub process {
> > >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
> > >  		}
> > >  
> > > +# dsb is too ARMish, and should usually be mb.
> > > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> > > +            WARN("ARM_BARRIER",
> > > +                 "Use of dsb is discouranged: prefer mb.\n" .
> > > +                 $herecurr);
> > > +		}
> > 
> > This patch is whitespace damaged with a spelling error.
> > 
> > Also, if this is reasonable test, and I don't know
> > that it is, it should be cc'd to the linux-arm list
> > linux-arm-kernel at lists.infradead.org
> > 
> > Also, I suggest 2 tests, one for .S files and
> > another for .[ch] files, and this be made specific
> > to arch/arm... files
> > 
> > Something like:
> > 
> > 	if ($realfile =~ @^arch/arm@ &&
> > 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
> > 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
> > 		WARN("ARM_DSB",
> > 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
> > 	}
> > 
> > ARM people, is this reasonable?
> 
> I don't think this is a big deal today. 
> 
> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
> 
> For code *ouside* of arch/{arm,arm64}, there are a number of cases where we
> want to use dsb(), e.g. when dealing with architectural drivers that require
> special barriers, or for common code shared across arm and arm64.
> 
> It doesn't look like this is a big problem today, anyhow:
> 
> [mark at salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch          
> drivers/irqchip/irq-armada-370-xp.c:    dsb();
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/power/reset/arm-versatile-reboot.c:     dsb();
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define dsb(a)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:     dsb(sy);         /* data barrier operation */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         dsb(sy);
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);

Thanks Mark.

So it seems this shouldn't be applied.

Re: [PATCH] checkpatch: Add exceptions for dsb keyword usage

[Joe Perches]

On Fri, 2018-07-06 at 06:45 +0100, Mark Rutland wrote:
> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
> > On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
> > > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
> > > is discouraged. However, there are exceptions when dsb is used in a
> > > variable or a function name. Exceptions are when 'dsb' is prefixed with
> > > class [-_>*\.] and/or suffixed with class [-_\.;].
> 
> This is a really confusing way of describing the match behaviour, and doesn't
> explain why this is a big problem.
> 
> In C it's either:
> 
> 	dsb()
> 	dsb(scope)	// e.g. dsb(ish)
> 
> ... where scope is [a-z]*.
> 
> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
> 
> > []
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > 
> > []
> > > @@ -5372,6 +5372,12 @@ sub process {
> > >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
> > >  		}
> > >  
> > > +# dsb is too ARMish, and should usually be mb.
> > > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
> > > +            WARN("ARM_BARRIER",
> > > +                 "Use of dsb is discouranged: prefer mb.\n" .
> > > +                 $herecurr);
> > > +		}
> > 
> > This patch is whitespace damaged with a spelling error.
> > 
> > Also, if this is reasonable test, and I don't know
> > that it is, it should be cc'd to the linux-arm list
> > linux-arm-kernel@lists.infradead.org
> > 
> > Also, I suggest 2 tests, one for .S files and
> > another for .[ch] files, and this be made specific
> > to arch/arm... files
> > 
> > Something like:
> > 
> > 	if ($realfile =~ @^arch/arm@ &&
> > 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
> > 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
> > 		WARN("ARM_DSB",
> > 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
> > 	}
> > 
> > ARM people, is this reasonable?
> 
> I don't think this is a big deal today. 
> 
> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
> 
> For code *ouside* of arch/{arm,arm64}, there are a number of cases where we
> want to use dsb(), e.g. when dealing with architectural drivers that require
> special barriers, or for common code shared across arm and arm64.
> 
> It doesn't look like this is a big problem today, anyhow:
> 
> [mark@salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch          
> drivers/irqchip/irq-armada-370-xp.c:    dsb();
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/power/reset/arm-versatile-reboot.c:     dsb();
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define dsb(a)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:     dsb(sy);         /* data barrier operation */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         dsb(sy);
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);

Thanks Mark.

So it seems this shouldn't be applied.

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[pheragu at codeaurora.org]

On 2018-07-05 22:52, Joe Perches wrote:
> On Fri, 2018-07-06 at 06:45 +0100, Mark Rutland wrote:
>> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
>> > On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
>> > > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
>> > > is discouraged. However, there are exceptions when dsb is used in a
>> > > variable or a function name. Exceptions are when 'dsb' is prefixed with
>> > > class [-_>*\.] and/or suffixed with class [-_\.;].
>> 
>> This is a really confusing way of describing the match behaviour, and 
>> doesn't
>> explain why this is a big problem.
>> 
>> In C it's either:
>> 
>> 	dsb()
>> 	dsb(scope)	// e.g. dsb(ish)
>> 
>> ... where scope is [a-z]*.
>> 
>> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
>> 
>> > []
>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> >
>> > []
>> > > @@ -5372,6 +5372,12 @@ sub process {
>> > >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>> > >  		}
>> > >
>> > > +# dsb is too ARMish, and should usually be mb.
>> > > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
>> > > +            WARN("ARM_BARRIER",
>> > > +                 "Use of dsb is discouranged: prefer mb.\n" .
>> > > +                 $herecurr);
>> > > +		}
>> >
>> > This patch is whitespace damaged with a spelling error.
>> >
>> > Also, if this is reasonable test, and I don't know
>> > that it is, it should be cc'd to the linux-arm list
>> > linux-arm-kernel at lists.infradead.org
>> >
>> > Also, I suggest 2 tests, one for .S files and
>> > another for .[ch] files, and this be made specific
>> > to arch/arm... files
>> >
>> > Something like:
>> >
>> > 	if ($realfile =~ @^arch/arm@ &&
>> > 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
>> > 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
>> > 		WARN("ARM_DSB",
>> > 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
>> > 	}
>> >
>> > ARM people, is this reasonable?
>> 
>> I don't think this is a big deal today.
>> 
>> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
>> 
>> For code *ouside* of arch/{arm,arm64}, there are a number of cases 
>> where we
>> want to use dsb(), e.g. when dealing with architectural drivers that 
>> require
>> special barriers, or for common code shared across arm and arm64.
>> 
>> It doesn't look like this is a big problem today, anyhow:
>> 
>> [mark at salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch
>> drivers/irqchip/irq-armada-370-xp.c:    dsb();
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
>> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
>> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
>> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
>> drivers/power/reset/arm-versatile-reboot.c:     dsb();
>> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
>> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
>> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
>> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define 
>> dsb(a)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:    
>>  dsb(sy);         /* data barrier operation */
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:        
>>  dsb(sy);
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> 
> Thanks Mark.
> 
> So it seems this shouldn't be applied.
Thanks Joe. I appreciate your feedback.

Re: [PATCH] checkpatch: Add exceptions for dsb keyword usage

[pheragu]

On 2018-07-05 22:52, Joe Perches wrote:
> On Fri, 2018-07-06 at 06:45 +0100, Mark Rutland wrote:
>> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
>> > On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
>> > > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
>> > > is discouraged. However, there are exceptions when dsb is used in a
>> > > variable or a function name. Exceptions are when 'dsb' is prefixed with
>> > > class [-_>*\.] and/or suffixed with class [-_\.;].
>> 
>> This is a really confusing way of describing the match behaviour, and 
>> doesn't
>> explain why this is a big problem.
>> 
>> In C it's either:
>> 
>> 	dsb()
>> 	dsb(scope)	// e.g. dsb(ish)
>> 
>> ... where scope is [a-z]*.
>> 
>> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
>> 
>> > []
>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> >
>> > []
>> > > @@ -5372,6 +5372,12 @@ sub process {
>> > >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>> > >  		}
>> > >
>> > > +# dsb is too ARMish, and should usually be mb.
>> > > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
>> > > +            WARN("ARM_BARRIER",
>> > > +                 "Use of dsb is discouranged: prefer mb.\n" .
>> > > +                 $herecurr);
>> > > +		}
>> >
>> > This patch is whitespace damaged with a spelling error.
>> >
>> > Also, if this is reasonable test, and I don't know
>> > that it is, it should be cc'd to the linux-arm list
>> > linux-arm-kernel@lists.infradead.org
>> >
>> > Also, I suggest 2 tests, one for .S files and
>> > another for .[ch] files, and this be made specific
>> > to arch/arm... files
>> >
>> > Something like:
>> >
>> > 	if ($realfile =~ @^arch/arm@ &&
>> > 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
>> > 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
>> > 		WARN("ARM_DSB",
>> > 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
>> > 	}
>> >
>> > ARM people, is this reasonable?
>> 
>> I don't think this is a big deal today.
>> 
>> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
>> 
>> For code *ouside* of arch/{arm,arm64}, there are a number of cases 
>> where we
>> want to use dsb(), e.g. when dealing with architectural drivers that 
>> require
>> special barriers, or for common code shared across arm and arm64.
>> 
>> It doesn't look like this is a big problem today, anyhow:
>> 
>> [mark@salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch
>> drivers/irqchip/irq-armada-370-xp.c:    dsb();
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
>> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
>> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
>> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
>> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
>> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
>> drivers/power/reset/arm-versatile-reboot.c:     dsb();
>> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
>> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
>> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
>> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define 
>> dsb(a)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:    
>>  dsb(sy);         /* data barrier operation */
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:        
>>  dsb(sy);
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do { 
>> debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
>> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> 
> Thanks Mark.
> 
> So it seems this shouldn't be applied.
Thanks Joe. I appreciate your feedback.

[PATCH] checkpatch: Add exceptions for dsb keyword usage

[pheragu at codeaurora.org]

On 2018-07-05 22:45, Mark Rutland wrote:
> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
>> On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
>> > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
>> > is discouraged. However, there are exceptions when dsb is used in a
>> > variable or a function name. Exceptions are when 'dsb' is prefixed with
>> > class [-_>*\.] and/or suffixed with class [-_\.;].
> 
> This is a really confusing way of describing the match behaviour, and 
> doesn't
> explain why this is a big problem.
> 
> In C it's either:
> 
> 	dsb()
> 	dsb(scope)	// e.g. dsb(ish)
> 
> ... where scope is [a-z]*.
> 
> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
> 
>> []
>> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> []
>> > @@ -5372,6 +5372,12 @@ sub process {
>> >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>> >  		}
>> >
>> > +# dsb is too ARMish, and should usually be mb.
>> > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
>> > +            WARN("ARM_BARRIER",
>> > +                 "Use of dsb is discouranged: prefer mb.\n" .
>> > +                 $herecurr);
>> > +		}
>> 
>> This patch is whitespace damaged with a spelling error.
>> 
>> Also, if this is reasonable test, and I don't know
>> that it is, it should be cc'd to the linux-arm list
>> linux-arm-kernel at lists.infradead.org
>> 
>> Also, I suggest 2 tests, one for .S files and
>> another for .[ch] files, and this be made specific
>> to arch/arm... files
>> 
>> Something like:
>> 
>> 	if ($realfile =~ @^arch/arm@ &&
>> 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
>> 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
>> 		WARN("ARM_DSB",
>> 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
>> 	}
>> 
>> ARM people, is this reasonable?
> 
> I don't think this is a big deal today.
> 
> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
> 
> For code *ouside* of arch/{arm,arm64}, there are a number of cases 
> where we
> want to use dsb(), e.g. when dealing with architectural drivers that 
> require
> special barriers, or for common code shared across arm and arm64.
> 
> It doesn't look like this is a big problem today, anyhow:
> 
> [mark at salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch
> drivers/irqchip/irq-armada-370-xp.c:    dsb();
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/power/reset/arm-versatile-reboot.c:     dsb();
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define 
> dsb(a)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:
>  dsb(sy);         /* data barrier operation */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         
> dsb(sy);
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> 
> Thanks,
> Mark.
Thanks Mark.  I appreciate your feedback.

Re: [PATCH] checkpatch: Add exceptions for dsb keyword usage

[pheragu]

On 2018-07-05 22:45, Mark Rutland wrote:
> On Thu, Jul 05, 2018 at 02:14:28PM -0700, Joe Perches wrote:
>> On Thu, 2018-07-05 at 11:19 -0700, Prakruthi Deepak Heragu wrote:
>> > mb() API can relpace the dsb() API in the kernel code. So, dsb() usage
>> > is discouraged. However, there are exceptions when dsb is used in a
>> > variable or a function name. Exceptions are when 'dsb' is prefixed with
>> > class [-_>*\.] and/or suffixed with class [-_\.;].
> 
> This is a really confusing way of describing the match behaviour, and 
> doesn't
> explain why this is a big problem.
> 
> In C it's either:
> 
> 	dsb()
> 	dsb(scope)	// e.g. dsb(ish)
> 
> ... where scope is [a-z]*.
> 
> ... which can be matched as something like 'dsb([a-z]*)' if necessary.
> 
>> []
>> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> []
>> > @@ -5372,6 +5372,12 @@ sub process {
>> >  			     "Avoid line continuations in quoted strings\n" . $herecurr);
>> >  		}
>> >
>> > +# dsb is too ARMish, and should usually be mb.
>> > +        if ($line =~ /[^-_>*\.]\bdsb\b[^-_\.;]/) {
>> > +            WARN("ARM_BARRIER",
>> > +                 "Use of dsb is discouranged: prefer mb.\n" .
>> > +                 $herecurr);
>> > +		}
>> 
>> This patch is whitespace damaged with a spelling error.
>> 
>> Also, if this is reasonable test, and I don't know
>> that it is, it should be cc'd to the linux-arm list
>> linux-arm-kernel@lists.infradead.org
>> 
>> Also, I suggest 2 tests, one for .S files and
>> another for .[ch] files, and this be made specific
>> to arch/arm... files
>> 
>> Something like:
>> 
>> 	if ($realfile =~ @^arch/arm@ &&
>> 	    ($realfile =~ /\.S$/ && $line =~ /\bdsb\b/) ||
>> 	    ($realfile =~ /\.[ch]$/ && $line =~ /\bdsb\s*\(/)) {
>> 		WARN("ARM_DSB",
>> 		     "Prefer mb over dsb as an ARM memory barrier\n" . $herecurr);
>> 	}
>> 
>> ARM people, is this reasonable?
> 
> I don't think this is a big deal today.
> 
> For code under arch/{arm,arm64}, it's perfectly reasonable to use dsb.
> 
> For code *ouside* of arch/{arm,arm64}, there are a number of cases 
> where we
> want to use dsb(), e.g. when dealing with architectural drivers that 
> require
> special barriers, or for common code shared across arm and arm64.
> 
> It doesn't look like this is a big problem today, anyhow:
> 
> [mark@salmiak:~/src/linux]% git grep -w 'dsb(.*)' -- ^arch
> drivers/irqchip/irq-armada-370-xp.c:    dsb();
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(ishst);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:               dsb(sy);
> drivers/irqchip/irq-gic-v3-its.c:       dsb(sy);
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/mtd/nand/raw/cmx270_nand.c:     dsb();
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/perf/arm_spe_pmu.c:     dsb(nsh);
> drivers/power/reset/arm-versatile-reboot.c:     dsb();
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/soc/rockchip/pm_domains.c:      dsb(sy);
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/mt7621-mmc/sd.c:        //dsb(); /* --- by chhung */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq.h:#define 
> dsb(a)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c:
>  dsb(sy);         /* data barrier operation */
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c:         
> dsb(sy);
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d] = __LINE__; dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d] = (v); dsb(sy); } while (0)
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h: do {
> debug_ptr[DEBUG_ ## d]++; dsb(sy); } while (0)
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> virt/kvm/arm/hyp/vgic-v3-sr.c:                  dsb(sy);
> 
> Thanks,
> Mark.
Thanks Mark.  I appreciate your feedback.