On Fri, 11 Aug 2023 13:02:35 +0200
Martin Botka <[email protected]> wrote:
Hi Martin,
> Add support for the H616 SID controller.
thanks for upstreaming this!
>
> The config can be reused from A64.
>
> Signed-off-by: Martin Botka <[email protected]>
> ---
> drivers/nvmem/sunxi_sid.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> index a970f1741cc6..df6fb5e0b724 100644
> --- a/drivers/nvmem/sunxi_sid.c
> +++ b/drivers/nvmem/sunxi_sid.c
> @@ -216,6 +216,7 @@ static const struct of_device_id sunxi_sid_of_match[] = {
> { .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
> { .compatible = "allwinner,sun50i-h5-sid", .data = &sun50i_a64_cfg },
> { .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg },
> + { .compatible = "allwinner,sun50i-h616-sid", .data = &sun50i_a64_cfg },
You don't need a new compatible string, then.
Just use
compatible = "allwinner,sun50i-h616-sid", "allwinner,sun50i-a64-sid";
in the DT, and add that combo to the binding, then you don't need any
driver patches at all - with the added benefit of that already working
with existing kernels.
Though I wonder if that has really only 256 bytes of fuses? Do we have any
evidence of that?
Cheers,
Andre
> {/* sentinel */},
> };
> MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
>
On Fri, Aug 11 2023 at 12:17:54 PM +01:00:00, Andre Przywara
<[email protected]> wrote:
> On Fri, 11 Aug 2023 13:02:35 +0200
> Martin Botka <[email protected]> wrote:
>
> Hi Martin,
>
>> Add support for the H616 SID controller.
>
> thanks for upstreaming this!
>
>>
>> The config can be reused from A64.
>>
>> Signed-off-by: Martin Botka <[email protected]>
>> ---
>> drivers/nvmem/sunxi_sid.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
>> index a970f1741cc6..df6fb5e0b724 100644
>> --- a/drivers/nvmem/sunxi_sid.c
>> +++ b/drivers/nvmem/sunxi_sid.c
>> @@ -216,6 +216,7 @@ static const struct of_device_id
>> sunxi_sid_of_match[] = {
>> { .compatible = "allwinner,sun50i-a64-sid", .data =
>> &sun50i_a64_cfg },
>> { .compatible = "allwinner,sun50i-h5-sid", .data =
>> &sun50i_a64_cfg },
>> { .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg
>> },
>> + { .compatible = "allwinner,sun50i-h616-sid", .data =
>> &sun50i_a64_cfg },
>
> You don't need a new compatible string, then.
> Just use
> compatible = "allwinner,sun50i-h616-sid",
> "allwinner,sun50i-a64-sid";
> in the DT, and add that combo to the binding, then you don't need any
> driver patches at all - with the added benefit of that already working
> with existing kernels.
I added it cause H5 and A83T were added after their DT binding was in
and they also use A64 config.
But I will drop this patch and use a64 compatible if that is the
prefered way :)
>
> Though I wonder if that has really only 256 bytes of fuses? Do we
> have any
> evidence of that?
Datasheet has 0 info regarding this. The way i got the size and offset
was to check the vendor code.
The values matched A64.
>
> Cheers,
> Andre
>
>> {/* sentinel */},
>> };
>> MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
>>
>
On Fri, 11 Aug 2023 15:12:34 +0200
Martin Botka <[email protected]> wrote:
Hi,
Maxime, Srinivas, can you possibly shed some light on the exact scope of
this nvmem DT binding? See below for more details ...
> On Fri, Aug 11 2023 at 12:17:54 PM +01:00:00, Andre Przywara
> <[email protected]> wrote:
> > On Fri, 11 Aug 2023 13:02:35 +0200
> > Martin Botka <[email protected]> wrote:
> >
> > Hi Martin,
> >
> >> Add support for the H616 SID controller.
> >
> > thanks for upstreaming this!
> >
> >>
> >> The config can be reused from A64.
> >>
> >> Signed-off-by: Martin Botka <[email protected]>
> >> ---
> >> drivers/nvmem/sunxi_sid.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> >> index a970f1741cc6..df6fb5e0b724 100644
> >> --- a/drivers/nvmem/sunxi_sid.c
> >> +++ b/drivers/nvmem/sunxi_sid.c
> >> @@ -216,6 +216,7 @@ static const struct of_device_id
> >> sunxi_sid_of_match[] = {
> >> { .compatible = "allwinner,sun50i-a64-sid", .data =
> >> &sun50i_a64_cfg },
> >> { .compatible = "allwinner,sun50i-h5-sid", .data =
> >> &sun50i_a64_cfg },
> >> { .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg
> >> },
> >> + { .compatible = "allwinner,sun50i-h616-sid", .data =
> >> &sun50i_a64_cfg },
> >
> > You don't need a new compatible string, then.
> > Just use
> > compatible = "allwinner,sun50i-h616-sid",
> > "allwinner,sun50i-a64-sid";
> > in the DT, and add that combo to the binding, then you don't need any
> > driver patches at all - with the added benefit of that already working
> > with existing kernels.
> I added it cause H5 and A83T were added after their DT binding was in
> and they also use A64 config.
Yeah, I saw this, and going back in the commit messages I found this:
"The H5 SoC have a SID controller that looks like the one in A64, the
cells are in the same offset but doesn't contain the same data (thermal
sensor calibration for example)."
So does the compatible string for the NVMEM *controller* really
encompass the location, encoding and meaning of all its NVMEM cells?
Isn't that described independently by the child nodes, that put names
to addresses, and that are referenced by other DT nodes?
Or was it originally not the case, and the scope changed over time?
It's certainly no biggie to use a single compatible string, and add a
single line to the driver (like in this patch), but it would be good to
know if this is really the intention.
Cheers,
Andre
> But I will drop this patch and use a64 compatible if that is the
> prefered way :)
> >
> > Though I wonder if that has really only 256 bytes of fuses? Do we
> > have any
> > evidence of that?
> Datasheet has 0 info regarding this. The way i got the size and offset
> was to check the vendor code.
> The values matched A64.
> >
> > Cheers,
> > Andre
> >
> >> {/* sentinel */},
> >> };
> >> MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
> >>
> >
>
>
>