Re: [PATCH v3 2/3] pinctrl: bcm: Add STB family pin controller driver

[Andrea della Porta <andrea.porta@suse.com>]

Hi Stanimir,

On 10:40 Tue 19 Aug     , Stanimir Varbanov wrote:
> Hi Andrea,
> 
> On 8/11/25 5:46 PM, Andrea della Porta wrote:
> > From: "Ivan T. Ivanov" <iivanov@suse.de>
> > 
> > This driver provide pin muxing and configuration functionality
> > for BCM2712 SoC used by RPi5. According to [1] this chip is an
> > instance of the one used in Broadcom STB  product line.
> > 
> > [1] https://lore.kernel.org/lkml/f6601f73-cb22-4ba3-88c5-241be8421fc3@broadcom.com/
> > 
> > Cc: Jonathan Bell <jonathan@raspberrypi.com>
> > Cc: Phil Elwell <phil@raspberrypi.com>
> > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> > Reviewed-by: Phil Elwell <phil@raspberrypi.com>
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> >  drivers/pinctrl/bcm/Kconfig           |   13 +
> >  drivers/pinctrl/bcm/Makefile          |    1 +
> >  drivers/pinctrl/bcm/pinctrl-brcmstb.c | 1197 +++++++++++++++++++++++++
> >  3 files changed, 1211 insertions(+)
> >  create mode 100644 drivers/pinctrl/bcm/pinctrl-brcmstb.c
> > 
> 
> <snip>
> 
> > +static int brcmstb_pinctrl_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	const struct brcmstb_pdata *pdata;
> > +	const struct of_device_id *match;
> > +	struct brcmstb_pinctrl *pc;
> > +	const char **names;
> > +	int num_pins, i;
> > +
> > +	match = of_match_node(brcmstb_pinctrl_match, np);
> 
> The 'match' variable is needless, you can drop it.

you mean something like this?

pdata = of_match_node(brcmstb_pinctrl_match, np)->data;

I thought that kind of compact code was not really the way to go,
at least taking a look at other driver exmaples: there's only one
avoiding the intermediate variable and many others using it (although
in some cases they also check for null, so in that case is fully
justified). Anyway, I've no preference on that so I can proceed with
your suggestion, unless anyone has something against it.

Many thanks,
Andrea

> 
> > +	pdata = match->data;
> > +
> 
> <snip>
> 
> ~Stan
>