Re: [PATCH 2/4] power: reset: at91-poweroff: move shdwc related data to one structure

[Sebastian Reichel]

Hi,

On Wed, Nov 07, 2018 at 06:23:40PM +0100, Alexandre Belloni wrote:
> On 07/11/2018 14:54:17+0000, Claudiu.Beznea@xxxxxxxxxxxxx wrote:
> > Hi Alexandre,
> > 
> > On 06.11.2018 23:09, Alexandre Belloni wrote:
> > > Hi Claudiu,
> > > 
> > > On 05/11/2018 11:14:26+0000, Claudiu.Beznea@xxxxxxxxxxxxx wrote:
> > >>  static int __init at91_poweroff_probe(struct platform_device *pdev)
> > >> @@ -154,16 +160,22 @@ static int __init at91_poweroff_probe(struct platform_device *pdev)
> > >>  	u32 ddr_type;
> > >>  	int ret;
> > >>  
> > >> +	at91_shdwc = devm_kzalloc(&pdev->dev, sizeof(*at91_shdwc), GFP_KERNEL);
> > >> +	if (!at91_shdwc)
> > >> +		return -ENOMEM;
> > >> +
> > > 
> > > Is there any real benefit that will offset the time lost for that
> > > allocation at boot time?
> > 
> > No, I haven't run benchmarks on this. I only wanted to have them grouped in
> > one structure. Please let me know if you have some tests in mind.
> > 
> 
> Well, it is probably not much but small things adds up. Having it as a
> global structure is probably good enough.

I suppose I will get a new patch with this change?

-- Sebastian

> 
> > > 
> > > I understand you are then testing at91_shdwc to know whether the driver
> > > already probed once. But, the driver will never probe twice as there is
> > > only one shutdown controller on the SoC and anyway, If it was to probe
> > > twice, it will still work as expected.
> > 
> > I had in mind the scenario where the driver would be compiled as module. I
> > know insmod already does this checking. I'm ok to remove this checking. I
> > will do it in next version. With this I will also remove devm_kzalloc() of
> > at91_shdwc.
> > 
> 
> Thanks,
> 
> -- 
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Attachment: signature.asc
Description: PGP signature