Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver

[Bjorn Andersson]

On Tue 08 Oct 16:45 PDT 2019, Stephen Boyd wrote:

> Now a two part series. These patches fix a debugfs name collision for
> the llcc regmaps and moves the config to a local variable to save on
> image size.
> 
> Changes from v1 (https://lkml.kernel.org/r/20191004233132.194336-1-swboyd@xxxxxxxxxxxx):
>  * New second patch
>  * Dropped static
>  * See range-diff below!
> 
> Stephen Boyd (2):
>   soc: qcom: llcc: Name regmaps to avoid collisions
>   soc: qcom: llcc: Move regmap config to local variable
> 
>  drivers/soc/qcom/llcc-slice.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> Cc: Venkata Narendra Kumar Gutta <vnkgutta@xxxxxxxxxxxxxx>
> Cc: Evan Green <evgreen@xxxxxxxxxxxx>
> 
> Range-diff against v1:
> -:  ------------ > 1:  07bc0e8bdb6e soc: qcom: llcc: Name regmaps to avoid collisions
> 1:  0c54fc8a7ed6 ! 2:  5c4446e36783 soc: qcom: llcc: Name regmaps to avoid collisions
>     @@ Metadata
>      Author: Stephen Boyd <swboyd@xxxxxxxxxxxx>
>      
>       ## Commit message ##
>     -    soc: qcom: llcc: Name regmaps to avoid collisions
>     +    soc: qcom: llcc: Move regmap config to local variable
>      
>     -    We'll end up with debugfs collisions if we don't give names to the
>     -    regmaps created inside this driver. Copy the template config over into
>     -    this function and give the regmap the same name as the resource name.
>     +    This is now a global variable that we're modifying to fix the name.
>     +    That isn't terribly thread safe and it's not necessary to be a global so
>     +    let's just move this to a local variable instead. This saves space in
>     +    the symtab and actually reduces kernel image size because the regmap
>     +    config is large and we can replace the initialization of that structure
>     +    with a memset and a few member assignments.
>      
>     -    Fixes: 7f9c136216c7 ("soc: qcom: Add broadcast base for Last Level Cache Controller (LLCC)")
>     -    Cc: Venkata Narendra Kumar Gutta <vnkgutta@xxxxxxxxxxxxxx>
>     -    Cc: Evan Green <evgreen@xxxxxxxxxxxx>
>          Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
>      
>       ## drivers/soc/qcom/llcc-slice.c ##
>     @@ drivers/soc/qcom/llcc-slice.c
>       
>       static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
>       
>     --static const struct regmap_config llcc_regmap_config = {
>     +-static struct regmap_config llcc_regmap_config = {
>      -	.reg_bits = 32,
>      -	.reg_stride = 4,
>      -	.val_bits = 32,
>     @@ drivers/soc/qcom/llcc-slice.c: static struct regmap *qcom_llcc_init_mmio(struct
>       {
>       	struct resource *res;
>       	void __iomem *base;
>     -+	static struct regmap_config llcc_regmap_config = {
>     ++	struct regmap_config llcc_regmap_config = {

Now that this isn't static I like the end result better. Not sure about
the need for splitting it in two patches, but if Evan is happy I'll take
it.

Regards,
Bjorn

>      +		.reg_bits = 32,
>      +		.reg_stride = 4,
>      +		.val_bits = 32,
>     @@ drivers/soc/qcom/llcc-slice.c: static struct regmap *qcom_llcc_init_mmio(struct
>       
>       	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
>       	if (!res)
>     -@@ drivers/soc/qcom/llcc-slice.c: static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
>     - 	if (IS_ERR(base))
>     - 		return ERR_CAST(base);
>     - 
>     -+	llcc_regmap_config.name = name;
>     - 	return devm_regmap_init_mmio(&pdev->dev, base, &llcc_regmap_config);
>     - }
>     - 
> 
> base-commit: 8b0eed9f6e36a5488967b0acc51444d658dd711b
> -- 
> Sent by a computer through tubes
>