Hi Geert,
geert@xxxxxxxxxxxxxx wrote on Thu, 24 Feb 2022 13:16:09 +0100:
> Hi Miquel,
>
> On Thu, Feb 24, 2022 at 12:36 PM Miquel Raynal
> <miquel.raynal@xxxxxxxxxxx> wrote:
> > > > > > > > +static int rzn1_dmamux_probe(struct platform_device *pdev)
> > > > > > > > +{
> > > > > > > > + struct device_node *mux_node = pdev->dev.of_node;
> > > > > > > > + const struct of_device_id *match;
> > > > > > > > + struct device_node *dmac_node;
> > > > > > > > + struct rzn1_dmamux_data *dmamux;
> > > > > > > > +
> > > > > > > > + dmamux = devm_kzalloc(&pdev->dev, sizeof(*dmamux), GFP_KERNEL);
> > > > > > > > + if (!dmamux)
> > > > > > > > + return -ENOMEM;
> > > > > > > > +
> > > > > > > > + mutex_init(&dmamux->lock);
> > > > > > > > +
> > > > > > > > + dmac_node = of_parse_phandle(mux_node, "dma-masters", 0);
> > > > > > > > + if (!dmac_node)
> > > > > > > > + return dev_err_probe(&pdev->dev, -ENODEV, "Can't get DMA master node\n");
> > > > > > > > +
> > > > > > > > + match = of_match_node(rzn1_dmac_match, dmac_node);
> > > > > > > > + if (!match) {
> > > > > > > > + of_node_put(dmac_node);
> > > > > > > > + return dev_err_probe(&pdev->dev, -EINVAL, "DMA master is not supported\n");
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + if (of_property_read_u32(dmac_node, "dma-requests", &dmamux->dmac_requests)) {
> > > > > > > > + of_node_put(dmac_node);
> > > > > > > > + return dev_err_probe(&pdev->dev, -EINVAL, "Missing DMAC requests information\n");
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + of_node_put(dmac_node);
> > > > > > >
> > > > > > > When hardcoding dmac_requests to 16, I guess the whole dmac_node
> > > > > > > handling can be removed?
> > > > > >
> > > > > > Not really, I think the following checks are still valid and fortunate,
> > > > > > and they need some of_ handling to work properly:
> > > > > > - verify that the chan requested is within the range of dmac_requests
> > > > > > in the _route_allocate() callback
> > > > > > - ensure the dmamux is wired to a supported DMAC in the DT (this
> > > > > > condition might be loosen in the future if needed or dropped entirely
> > > > > > if considered useless)
> > > > > > - I would like to add a check against the number of requests supported
> > > > > > by the dmamux and the dmac (not done yet).
> > > > > > For the record, I've taken inspiration to write these lines on the other
> > > > > > dma router driver from TI.
> >
> > ^^^^^^^^^^^
> > ... these checks
>
> I don't know. Some of them will be checked when calling into the
> parent DMAC, right?
Only the first item above will be validated by the DMAC driver. But I
prefer to error out sooner than later, because getting the mux in
place while knowing that the request is invalid sounds silly.
Thanks,
Miquèl
