On Tue, Mar 26, 2024 at 12:31:18PM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 12:15, Inochi Amaoto wrote:
> > On Tue, Mar 26, 2024 at 09:53:09AM +0100, Krzysztof Kozlowski wrote:
> >> On 26/03/2024 08:35, Inochi Amaoto wrote:
> >>>>> +
> >>>>> +required:
> >>>>> + - '#dma-cells'
> >>>>> + - dma-masters
> >>>>> +
> >>>>
> >>>>
> >>>> I don't understand what happened here. Previously you had a child and I
> >>>> proposed to properly describe it with $ref.
> >>>>
> >>>> Now, all children are gone. Binding is supposed to be complete. Based on
> >>>> your cover letter, this is not complete, but why? What is missing and
> >>>> why it cannot be added?
> >>>>
> >>>
> >>> The binding of syscon is removed due to a usb phy subdevices, which needs
> >>> sometime to figure out the actual property. This is why the syscon binding
> >>> is removed.
> >>>
> >>> I think it is better to use the origianl syscon series to evolve after
> >>> the usb phy binding is submitted. The subdevices of syscon may need
> >>> much reverse engineering to know its parameters. So at least for now,
> >>> the syscon binding is hard to be complete.
> >>
> >> Some explanation why dma-router is gone would be useful, but fine.
> >>
> >
> > OK, I will add some comments on the why it is gone.
> >
> >>>
> >>>>
> >>>>> +additionalProperties: false
> >>>>> +
> >>>>> +examples:
> >>>>> + - |
> >>>>> + dma-router {
> >>>>> + compatible = "sophgo,cv1800-dmamux";
> >>>>> + #dma-cells = <2>;
> >>>>> + dma-masters = <&dmac>;
> >>>>> + dma-requests = <8>;
> >>>>> + };
> >>>>> diff --git a/include/dt-bindings/dma/cv1800-dma.h b/include/dt-bindings/dma/cv1800-dma.h
> >>>>> new file mode 100644
> >>>>> index 000000000000..3ce9dac25259
> >>>>> --- /dev/null
> >>>>> +++ b/include/dt-bindings/dma/cv1800-dma.h
> >>>>
> >>>> Filename should match bindings filename.
> >>>>
> >>>
> >>> Thanks.
> >>>
> >>>>
> >>>> Anyway, the problem is that it is a dead header. I don't see it being
> >>>> used, so it is not a binding.
> >>>>
> >>>
> >>> This header is not used because the dmamux node is not defined at now.
> >>
> >> In the driver? The binding header is supposed to be used in the driver,
> >> otherwise it is not a binding.
> >>
> >
> > The driver does use this file.
>
> I checked and could not find. Please point me to specific parts of the code.
>
In cv1800_dmamux_route_allocate.
>+ regmap_set_bits(dmamux->regmap,
>+ DMAMUX_CH_REG(chid),
>+ DMAMUX_CH_SET(chid, devid));
>+
>+ regmap_update_bits(dmamux->regmap, CV1800_SDMA_DMA_INT_MUX,
>+ DMAMUX_INT_CH_MASK(chid, cpuid),
>+ DMAMUX_INT_CH_BIT(chid, cpuid));
I think this is.
> >
> >>> And considering the limitation of this dmamux, maybe only devices that
> >>> require dma as a must can have the dma assigned.
> >>> Due to the fact, I think it may be a long time to wait for this header
> >>> to be used as the binding header.
> >>
> >> I don't understand. You did not provide a single reason why this is a
> >> binding. Reason is: mapping IDs between DTS and driver. Where is this
> >> reason?
> >>
> >
> > It seems like that I misunderstood something. This file provides one-one
> > mapping between the dma device id and cpuid, which is both used in the
> > dts and driver. For dts, it provides device id and cpu id mapping. And
> > for driver, it is used as the directive to tell how to write the mapping
> > register.
>
> So where is it? I looked for DMA_TDM0_RX - nothing. Then DMA_I2C1_RX -
> nothing. Then any "DMA_" - also looks nothing.
>
It is just the value writed, so I say it is just a one-one mapping.
Maybe I misunderstand the binding meaning? Is the binding a mapping
between the dts and something defind in the driver (not the real
device)?
Regards,
Inochi.
