| Message ID | 8a8d5ed47f24791d3927345fafed07023a8b0b76.1688032865.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix violations of MISRA C:2012 Rule 3.1 | expand |
> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: > > In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences here you use a different character to enclose the file path (` vs ‘) may I suggest to use only (‘)? > of nested '//' character sequences inside C-style comment blocks, which violate > Rule 3.1. > > The patch aims to resolve those by replacing the nested comments with > equivalent constructs that do not violate the rule. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> You are missing the “---“ here, meaning that the lines below are part of the commit message and I’m sure you don’t want that. Also here, may I suggest to use this commit title instead? “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1” Apart from that, changes looks good to me: Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Will be the maintainer/committer to decide if addressing these comment, if accepted, on commit or if you need to send another version, in which case you can retain my r-by provided that no other modifications are done. > Changes: > - Resending the patch with the right maintainers in CC. > Changes in V2: > - Split the patch into a series and reworked the fix. > - Apply the fix to the arm32 `flushtlb.h' file, for consistency > Changes in V3: > - Revised the comment to make it clear the function the parallel control > flows in the comment belong to. > --- > xen/drivers/passthrough/arm/smmu-v3.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > index 720aa69ff2..cdbb505134 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, > * before we read 'nr_ats_masters' in case of a concurrent call to > * arm_smmu_enable_ats(): > * > - * // unmap() // arm_smmu_enable_ats() > - * TLBI+SYNC atomic_inc(&nr_ats_masters); > - * smp_mb(); [...] > - * atomic_read(&nr_ats_masters); pci_enable_ats() // writel() > + * --- unmap() --- --- arm_smmu_enable_ats() --- > + * TLBI+SYNC atomic_inc(&nr_ats_masters); > + * smp_mb(); [...] > + * atomic_read(&nr_ats_masters); pci_enable_ats() (see writel()) > * > * Ensures that we always see the incremented 'nr_ats_masters' count if > * ATS was enabled at the PCI device before completion of the TLBI. > -- > 2.34.1 > >
On Thu, 29 Jun 2023, Luca Fancellu wrote: > > On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: > > > > In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences > > here you use a different character to enclose the file path (` vs ‘) may I suggest to > use only (‘)? > > > of nested '//' character sequences inside C-style comment blocks, which violate > > Rule 3.1. > > > > The patch aims to resolve those by replacing the nested comments with > > equivalent constructs that do not violate the rule. > > > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > > You are missing the “---“ here, meaning that the lines below are part of the > commit message and I’m sure you don’t want that. > > Also here, may I suggest to use this commit title instead? > “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1” > > Apart from that, changes looks good to me: > > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > > Will be the maintainer/committer to decide if addressing these comment, > if accepted, on commit or if you need to send another version, in which > case you can retain my r-by provided that no other modifications are done. I think it can be done on commit. With the changes suggested by Luca: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > Changes: > > - Resending the patch with the right maintainers in CC. > > Changes in V2: > > - Split the patch into a series and reworked the fix. > > - Apply the fix to the arm32 `flushtlb.h' file, for consistency > > Changes in V3: > > - Revised the comment to make it clear the function the parallel control > > flows in the comment belong to. > > --- > > xen/drivers/passthrough/arm/smmu-v3.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > > index 720aa69ff2..cdbb505134 100644 > > --- a/xen/drivers/passthrough/arm/smmu-v3.c > > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > > @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, > > * before we read 'nr_ats_masters' in case of a concurrent call to > > * arm_smmu_enable_ats(): > > * > > - * // unmap() // arm_smmu_enable_ats() > > - * TLBI+SYNC atomic_inc(&nr_ats_masters); > > - * smp_mb(); [...] > > - * atomic_read(&nr_ats_masters); pci_enable_ats() // writel() > > + * --- unmap() --- --- arm_smmu_enable_ats() --- > > + * TLBI+SYNC atomic_inc(&nr_ats_masters); > > + * smp_mb(); [...] > > + * atomic_read(&nr_ats_masters); pci_enable_ats() (see writel()) > > * > > * Ensures that we always see the incremented 'nr_ats_masters' count if > > * ATS was enabled at the PCI device before completion of the TLBI. > > -- > > 2.34.1 > > > > > >
On 29.06.2023 16:52, Luca Fancellu wrote: > > >> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >> >> In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences > > here you use a different character to enclose the file path (` vs ‘) may I suggest to > use only (‘)? > >> of nested '//' character sequences inside C-style comment blocks, which violate >> Rule 3.1. >> >> The patch aims to resolve those by replacing the nested comments with >> equivalent constructs that do not violate the rule. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > > You are missing the “---“ here, meaning that the lines below are part of the > commit message and I’m sure you don’t want that. > > Also here, may I suggest to use this commit title instead? > “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1” Just to mention it: Personally I'm averse to such double subject prefixes. Why would (here) "xen/smmuv3: " not be sufficient (and entirely unambiguous)? Jan
Hi, > On 4 Jul 2023, at 4:51 pm, Jan Beulich <jbeulich@suse.com> wrote: > > On 29.06.2023 16:52, Luca Fancellu wrote: >> >> >>> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>> >>> In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences >> >> here you use a different character to enclose the file path (` vs ‘) may I suggest to >> use only (‘)? >> >>> of nested '//' character sequences inside C-style comment blocks, which violate >>> Rule 3.1. >>> >>> The patch aims to resolve those by replacing the nested comments with >>> equivalent constructs that do not violate the rule. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> >> You are missing the “---“ here, meaning that the lines below are part of the >> commit message and I’m sure you don’t want that. >> >> Also here, may I suggest to use this commit title instead? >> “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1” > > Just to mention it: Personally I'm averse to such double subject prefixes. > Why would (here) "xen/smmuv3: " not be sufficient (and entirely unambiguous)? With the changes suggested above. Acked-by: Rahul Singh <rahul.singh@arm.com> Regards, Rahul
Hi Rahul, On 05/07/2023 09:40, Rahul Singh wrote: >> On 4 Jul 2023, at 4:51 pm, Jan Beulich <jbeulich@suse.com> wrote: >> >> On 29.06.2023 16:52, Luca Fancellu wrote: >>> >>> >>>> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@bugseng.com> wrote: >>>> >>>> In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences >>> >>> here you use a different character to enclose the file path (` vs ‘) may I suggest to >>> use only (‘)? >>> >>>> of nested '//' character sequences inside C-style comment blocks, which violate >>>> Rule 3.1. >>>> >>>> The patch aims to resolve those by replacing the nested comments with >>>> equivalent constructs that do not violate the rule. >>>> >>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >>> >>> You are missing the “---“ here, meaning that the lines below are part of the >>> commit message and I’m sure you don’t want that. >>> >>> Also here, may I suggest to use this commit title instead? >>> “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1” >> >> Just to mention it: Personally I'm averse to such double subject prefixes. >> Why would (here) "xen/smmuv3: " not be sufficient (and entirely unambiguous)? > > With the changes suggested above. There are conflicting suggestions about the title. So it is not clear to me which one you are referring to. I will assume you were happy either way and so picked Luca's proposal and committed. > > Acked-by: Rahul Singh <rahul.singh@arm.com> > > Regards, > Rahul > >
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 720aa69ff2..cdbb505134 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, * before we read 'nr_ats_masters' in case of a concurrent call to * arm_smmu_enable_ats(): * - * // unmap() // arm_smmu_enable_ats() - * TLBI+SYNC atomic_inc(&nr_ats_masters); - * smp_mb(); [...] - * atomic_read(&nr_ats_masters); pci_enable_ats() // writel() + * --- unmap() --- --- arm_smmu_enable_ats() --- + * TLBI+SYNC atomic_inc(&nr_ats_masters); + * smp_mb(); [...] + * atomic_read(&nr_ats_masters); pci_enable_ats() (see writel()) * * Ensures that we always see the incremented 'nr_ats_masters' count if * ATS was enabled at the PCI device before completion of the TLBI.
In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences of nested '//' character sequences inside C-style comment blocks, which violate Rule 3.1. The patch aims to resolve those by replacing the nested comments with equivalent constructs that do not violate the rule. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> Changes: - Resending the patch with the right maintainers in CC. Changes in V2: - Split the patch into a series and reworked the fix. - Apply the fix to the arm32 `flushtlb.h' file, for consistency Changes in V3: - Revised the comment to make it clear the function the parallel control flows in the comment belong to. --- xen/drivers/passthrough/arm/smmu-v3.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)