Re: [PATCH v2 4/8] dma: dmamux: Introduce RZN1 DMA router support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

> > > > > > +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

> > > >
> > > > Unless, and I know some people think like that, we do not try to
> > > > validate the DT and if the DT is wrong that is none of our business.
> > > >  
> > > > >  
> > > > > > +
> > > > > > +       if (of_property_read_u32(mux_node, "dma-requests", &dmamux->dmamux_requests)) {  
> > > > >
> > > > > Don't obtain from DT, but fix to 32?  
> > > >
> > > > I believe the answer to the previous question should give me a clue
> > > > about why you would prefer hardcoding than reading from the DT such
> > > > an information. Perhaps I should mention that all these properties are
> > > > already part of the bindings, and are not specific to the driver, the
> > > > information will be in the DT anyway.  
> > >
> > > The 32 is a property of the hardware (32 bits in DMAMUX register).
> > > So IMHO it falls under the "differentiate by compatible value,
> > > not by property" rule.  
> >
> > I agree this is a property of the hardware and feels redundant here.
> >
> > What about the checks below, do you agree with the fact that I should
> > keep them or do you prefer dropping them (all? partially?)?  
> 
> There are no checks below?

I meant above /o\ ...

> /me confused.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux