Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 18 Nov 2022 01:12:32 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Umang Jain <umang.jain@ideasonboard.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Kieran Bingham <kieran.bingham@ideasonboard.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Dan Carpenter <error27@gmail.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        Ray Jui <rjui@broadcom.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-rpi-kernel@lists.infradead.org, linux-staging@lists.linux.dev
Subject: Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in
 structures"
Message-ID: <Y3bN8I7DlxRUZumH@lunn.ch>
References: <20221117160015.344528-1-umang.jain@ideasonboard.com>
 <20221117160015.344528-2-umang.jain@ideasonboard.com>
 <166870135963.50677.14827688186331561108@Monstersaurus>
 <c718624a-bb6f-5474-5cc3-4319b1fdb282@ideasonboard.com>
 <Y3Z3Is8u4wGZfKU5@kroah.com>
 <c5e03c65-2994-6518-c79d-f6ad1557c4e0@ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c5e03c65-2994-6518-c79d-f6ad1557c4e0@ideasonboard.com>
Precedence: bulk

> > > > >    struct vchiq_mmal_port {
> > > > > -       u32 enabled:1;
> > > > > +       bool enabled:1;
> > > > Is this a direct revert with 'git revert' ?
> > > 
> > > No. It had conflicts plus I added the ':1' initialization to keep the logic
> > > same (in case 'enabled' gets used directly). Similar pattern come up with:
> > >  ??? ($) git grep 'bool' -- '*.[h]' | grep '\:1'
> > > 
> > > So it shouldn't be an issue.
> > Please don't do that "bool foo:1" makes no sense.  Drop the ":1"
> > please.
> 
> It won't affect this patch but if you take a look at 2/3 - you'll see a bool
> flag 'in_use' that needs to be initialized (as it's getting used directly).
> 
> I can move the initialization part in the function (_init() or something)
> and drop the ":1" as you mentioned. That's? fine as well but I do find
> patterns of 'bool foo:1' in the codebase so I assumed it would be safe to
> use.

Does :1 really initialise the variable? In "u32 enabled:1" it means
this is a 1 bit wide bit field. It seems odd that bool is somehow
special and :1 means something else.

	Andrew