Re: [Patch v6 1/4] memory: tegra: Add memory controller channels support

[Ashish Mhetre]

On 4/11/2022 12:03 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> On 4/11/22 09:05, Ashish Mhetre wrote:
> > On 4/10/2022 7:48 PM, Dmitry Osipenko wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > 06.04.2022 08:24, Ashish Mhetre пишет:
> > > > +     num_dt_channels =
> > > > of_property_count_elems_of_size(pdev->dev.of_node, "reg",
> > > > +                                                       reg_cells *
> > > > sizeof(u32));
> > > > +     /*
> > > > +      * On tegra186 onwards, memory controller support multiple
> > > > channels.
> > > > +      * Apart from regular memory controller channels, there is one
> > > > broadcast
> > > > +      * channel and one for stream-id registers.
> > > > +      */
> > > > +     if (num_dt_channels < mc->soc->num_channels + 2) {
> > > > +             dev_warn(&pdev->dev, "MC channels are missing, please
> > > > update memory controller DT node with MC channels\n");
> > > > +             return 0;
> > > > +     }
> > > > +
> > > > +     mc->bcast_ch_regs = devm_platform_ioremap_resource_byname(pdev,
> > > > "mc-broadcast");
> > > > +     if (IS_ERR(mc->bcast_ch_regs))
> > > > +             return PTR_ERR(mc->bcast_ch_regs);
> > >
> > > Looks to me that you don't need to use of_property_count_elems_of_size()
> > > and could only check the "mc-broadcast" presence to decide whether this
> > > is an older DT.
> > Now that we are using reg-names in new DT, yes it'd be fine to just
> > check mc-broadcast to decide it's a new or old DT.
> >
> > > mc->bcast_ch_regs = devm_platform_ioremap_resource_byname(pdev,
> > > "broadcast");
> > > if (IS_ERR(mc->bcast_ch_regs)) {
> > >           dev_warn(&pdev->dev, "Broadcast channel is missing, please
> > > update your
> > > device-tree\n");
> > >           return PTR_ERR(mc->bcast_ch_regs);
> > > }
> >
> > return 0;
> >
> > to avoid DT ABI break, right?
>
> Yes, it should be "return 0".

But if we "return 0" from here, then what about the case when ioremap()
actually fails with new DT i.e. when broadcast reg is present in DT?
In that case error should be returned and probe should be failed, right?