This adds a DT binding for the U8500 SRAMs.
This patch series also fixes an issue with pool labels that I
saw on U8500.
I suppose SRAM patches will go in through the SoC tree, I kind
of feel that SRAM is a SoC concept and the driver should
actually be in drivers/soc...
Signed-off-by: Linus Walleij <[email protected]>
---
Changes in v2:
- Change to just use dev_name() for naming the SRAM partition
when no label is passed.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Linus Walleij (2):
dt-bindings: sram: Add compatible for ST-Ericsson U8500 eSRAM
misc: sram: Generate unique names for subpools
Documentation/devicetree/bindings/sram/sram.yaml | 1 +
drivers/misc/sram.c | 9 +++++----
2 files changed, 6 insertions(+), 4 deletions(-)
---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230417-ux500-sram-961796726db4
Best regards,
--
Linus Walleij <[email protected]>
The current code will, if we do not specify unique labels
for the SRAM subnodes, fail to register several nodes named
the same.
Example:
sram@40020000 {
(...)
sram@0 {
(...)
};
sram@1000 {
(...)
};
};
Since the child->name in both cases will be "sram" the
gen_pool_create() will fail because the name is not unique.
Use dev_name() for the device as this will have bus ID
set to the fully translated address for the node, and that
will always be unique.
Signed-off-by: Linus Walleij <[email protected]>
---
ChangeLog v1->v2:
- Stop complicating things and just use dev_name()
---
drivers/misc/sram.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index f0e7f02605eb..f80c3adddf0b 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -240,10 +240,11 @@ static int sram_reserve_regions(struct sram_dev *sram, struct resource *res)
goto err_chunks;
}
if (!label)
- label = child->name;
-
- block->label = devm_kstrdup(sram->dev,
- label, GFP_KERNEL);
+ block->label = devm_kasprintf(sram->dev, GFP_KERNEL,
+ "%s", dev_name(sram->dev));
+ else
+ block->label = devm_kstrdup(sram->dev,
+ label, GFP_KERNEL);
if (!block->label) {
ret = -ENOMEM;
goto err_chunks;
--
2.40.0
21.04.2023 00:17, Linus Walleij пишет:
> The current code will, if we do not specify unique labels
> for the SRAM subnodes, fail to register several nodes named
> the same.
>
> Example:
>
> sram@40020000 {
> (...)
> sram@0 {
> (...)
> };
> sram@1000 {
> (...)
> };
> };
>
> Since the child->name in both cases will be "sram" the
> gen_pool_create() will fail because the name is not unique.
>
> Use dev_name() for the device as this will have bus ID
> set to the fully translated address for the node, and that
> will always be unique.
>
> Signed-off-by: Linus Walleij <[email protected]>
> ---
> ChangeLog v1->v2:
> - Stop complicating things and just use dev_name()
> ---
> drivers/misc/sram.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
> index f0e7f02605eb..f80c3adddf0b 100644
> --- a/drivers/misc/sram.c
> +++ b/drivers/misc/sram.c
> @@ -240,10 +240,11 @@ static int sram_reserve_regions(struct sram_dev *sram, struct resource *res)
> goto err_chunks;
> }
> if (!label)
> - label = child->name;
> -
> - block->label = devm_kstrdup(sram->dev,
> - label, GFP_KERNEL);
> + block->label = devm_kasprintf(sram->dev, GFP_KERNEL,
> + "%s", dev_name(sram->dev));
This broke device-trees that have no label property. The SRAM DT binding
says:
"
label:
description:
The name for the reserved partition, if omitted, the label is taken
from the node name excluding the unit address.
"
Not sure whether breakage was on purpose, otherwise doc needs to be
updated or there should be explicit check for the duplicated node names.
Secondly, AFAICS, the dev_name(sram->dev) is the name of the parent SRAM
device and not of the children sub-nodes, hence it's now always the same
dev_name(sram->dev) for all sub-nodes.
On Sun, Jun 18, 2023 at 11:33 PM Dmitry Osipenko <[email protected]> wrote:
> > if (!label)
> > - label = child->name;
> > -
> > - block->label = devm_kstrdup(sram->dev,
> > - label, GFP_KERNEL);
> > + block->label = devm_kasprintf(sram->dev, GFP_KERNEL,
> > + "%s", dev_name(sram->dev));
>
> This broke device-trees that have no label property.
Which system is affected? Asking so I can inspect the DTS file
and figure out how this needs to work.
> The SRAM DT binding says:
>
> "
> label:
> description:
> The name for the reserved partition, if omitted, the label is taken
> from the node name excluding the unit address.
> "
>
> Not sure whether breakage was on purpose, otherwise doc needs to be
> updated or there should be explicit check for the duplicated node names.
>
> Secondly, AFAICS, the dev_name(sram->dev) is the name of the parent SRAM
> device and not of the children sub-nodes, hence it's now always the same
> dev_name(sram->dev) for all sub-nodes.
Sounds like I should go back to the original approach in patch v1:
https://lore.kernel.org/linux-devicetree/[email protected]/
and also augment the DTS binding text to say it uses the full node name
including the address.
Does that look OK to you, or will this regress your system as well?
Yours,
Linus Walleij
19.06.2023 10:11, Linus Walleij пишет:
> On Sun, Jun 18, 2023 at 11:33 PM Dmitry Osipenko <[email protected]> wrote:
>
>>> if (!label)
>>> - label = child->name;
>>> -
>>> - block->label = devm_kstrdup(sram->dev,
>>> - label, GFP_KERNEL);
>>> + block->label = devm_kasprintf(sram->dev, GFP_KERNEL,
>>> + "%s", dev_name(sram->dev));
>>
>> This broke device-trees that have no label property.
>
> Which system is affected? Asking so I can inspect the DTS file
> and figure out how this needs to work.
NVIDIA Tegra2/3 video decoder driver fails to probe with this change.
https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/nvidia/tegra-vde/vde.c#L312
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/tegra20.dtsi#L347
>> The SRAM DT binding says:
>>
>> "
>> label:
>> description:
>> The name for the reserved partition, if omitted, the label is taken
>> from the node name excluding the unit address.
>> "
>>
>> Not sure whether breakage was on purpose, otherwise doc needs to be
>> updated or there should be explicit check for the duplicated node names.
>>
>> Secondly, AFAICS, the dev_name(sram->dev) is the name of the parent SRAM
>> device and not of the children sub-nodes, hence it's now always the same
>> dev_name(sram->dev) for all sub-nodes.
>
> Sounds like I should go back to the original approach in patch v1:
> https://lore.kernel.org/linux-devicetree/[email protected]/
>
> and also augment the DTS binding text to say it uses the full node name
> including the address.
>
> Does that look OK to you, or will this regress your system as well?
That may work, but then seems you'll also need to update
of_gen_pool_get() to use np_pool->full_name instead of np_pool->name.
https://elixir.bootlin.com/linux/latest/source/lib/genalloc.c#L898