Re: [PATCH v5 1/3] iommu/arm-smmu-v3: Introduce struct arm_smmu_event

[Pranjal Shrivastava <praan@google.com>]

On Tue, Nov 19, 2024 at 09:03:08AM +0000, Will Deacon wrote:
> On Mon, Nov 18, 2024 at 12:24:42PM -0400, Jason Gunthorpe wrote:
> > On Fri, Nov 15, 2024 at 01:34:12PM -0800, Nicolin Chen wrote:
> > > On Fri, Nov 15, 2024 at 01:13:36PM +0000, Pranjal Shrivastava wrote:
> > > > On Tue, Nov 12, 2024 at 03:30:58PM -0800, Nicolin Chen wrote:
> > > > > On Tue, Nov 12, 2024 at 08:30:16AM +0000, Pranjal Shrivastava wrote:
> > > > > > +struct arm_smmu_event {
> > > > > > +       u8                              stall           : 1,
> > > > > > +                                       ssv             : 1,
> > > > > > +                                       privileged      : 1,
> > > > > > +                                       instruction     : 1,
> > > > > > +                                       s2              : 1,
> > > > > > +                                       read            : 1;
> > > > > 
> > > > > Feels odd to do so...should all of them be:
> > > > > 	u8			name : 1;
> > > > > ?
> > > > 
> > > > Hmmm, do you mean something like the following?
> > > 
> > > Never mind. I see amd_iommu_types.h does something just like that.
> > 
> > I certainly expect to see the u8 repeated and not use , for members of
> > a structure. Also don't add excessive horizonal alignment.. C code is
> > not excel. :)
> 
> I think the comma syntax is fine (there's precedence for it and it works),
> but agreed on removing some of the tabs.

Ohh alright. I'm assuming both of you are referring to the tabs before
the colon ':' ? 

As in, is the following fine?

struct arm_smmu_event {
       u8		stall : 1,
			ssv : 1,
			privileged : 1,
			instruction : 1,
			s2 : 1,
			read : 1;
	.....
};


> 
> Will

Thanks,
Praan