linux-kernel - Re: [PATCH v2 09/10] soc: aspeed: lpc-snoop: Consolidate channel initialisation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6ae95d064de0c1b6333c234e461eae3e8da80168.camel@codeconstruct.com.au>
Date: Tue, 08 Jul 2025 11:36:59 +0930
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Jean Delvare <jdelvare@...e.de>
Cc: linux-aspeed@...ts.ozlabs.org, Joel Stanley <joel@....id.au>, Henry
 Martin <bsdhenrymartin@...il.com>, Patrick Rudolph
 <patrick.rudolph@...ements.com>,  Andrew Geissler <geissonator@...oo.com>,
 Ninad Palsule <ninad@...ux.ibm.com>, Patrick Venture <venture@...gle.com>, 
 Robert Lippert <roblip@...il.com>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/10] soc: aspeed: lpc-snoop: Consolidate channel
 initialisation

Hi Jean,

On Fri, 2025-07-04 at 17:13 +0200, Jean Delvare wrote:

> 
> > Signed-off-by: Andrew Jeffery <andrew@...econstruct.com.au>
> > ---
> >  drivers/soc/aspeed/aspeed-lpc-snoop.c | 51 +++++++++++++++++------------------
> >  1 file changed, 24 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c b/drivers/soc/aspeed/aspeed-lpc-snoop.c
> > index 8dbc9d4158b89f23bda340f060d205a29bbb43c3..9f88c5471b1b6d85f6d9e1970240f3d1904d166c 100644
> > --- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
> > +++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
> > @@ -294,12 +294,21 @@ static void aspeed_lpc_disable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
> >         kfifo_free(&channel->fifo);
> >  }
> >  
> > +static void aspeed_lpc_snoop_remove(struct platform_device *pdev)
> > +{
> > +       struct aspeed_lpc_snoop *lpc_snoop = dev_get_drvdata(&pdev->dev);
> > +
> > +       /* Disable both snoop channels */
> > +       aspeed_lpc_disable_snoop(lpc_snoop, ASPEED_LPC_SNOOP_INDEX_0);
> > +       aspeed_lpc_disable_snoop(lpc_snoop, ASPEED_LPC_SNOOP_INDEX_1);
> 
> For consistency with the probe function, I think it would make sense to
> use a for loop here as well, instead of hard-coding the channel number
> to 2. That way, no change will be needed if a future device supports
> more than 2 channels.

You're right, but for now I'm not bothered by it. I'm going to leave it
as is, as the motivation for the loop in the probe() path was to
consolidate the logic required for both channels. This one is an easy
thing to fix down the track.

> 
> None if this is blocking though, so:
> 
> Acked-by: Jean Delvare <jdelvare@...e.de>
> 

Thanks.

Andrew