Re: [PATCH 07/11] mfd: bcm2835-pm: Add support for BCM2711

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

 



Hi Stefan,

> Am 31.05.22 um 22:11 schrieb Peter Robinson:
> > Hi Stefan,
> >
> > On Tue, May 31, 2022 at 6:27 PM Stefan Wahren <stefan.wahren@xxxxxxxx> wrote:
> >> Hi Peter,
> >>
> >> Am 31.05.22 um 17:08 schrieb Peter Robinson:
> >>> On Sun, May 15, 2022 at 9:21 PM Stefan Wahren <stefan.wahren@xxxxxxxx> wrote:
> >>>> In BCM2711 the new RPiVid ASB took over V3D. The old ASB is still present
> >>>> with the ISP and H264 bits, and V3D is in the same place in the new ASB
> >>>> as the old one.
> >>>>
> >>>> As per the devicetree bindings, BCM2711 will provide both the old and
> >>>> new ASB resources, so get both of them and pass them into
> >>>> 'bcm2835-power,' which will take care of selecting which one to use
> >>>> accordingly.
> >>>>
> >>>> Since the RPiVid ASB's resources were being provided prior to formalizing
> >>>> the bindings[1], also support the old firmwares that didn't use
> >>> I'm guessing this [1] is referring to "[1] See: 7dbe8c62ceeb ("ARM:
> >>> dts: Add minimal Raspberry Pi 4 support")" referred to in the original
> >>> patch [1] that Nicolas did, was there a reason to drop the
> >>> details/changelog here?
> >> Oops, the link accidently get lost.
> >>> The decision not to use bits makes sense I
> >>> believe.
> >> Yes, i think the new version is more elegant.
> >>> [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220213225646.67761-8-pbrobinson@xxxxxxxxx/
> >>>
> >>>> 'reg-names.'
> >>>>
> >>>> Signed-off-by: Stefan Wahren <stefan.wahren@xxxxxxxx>
> >>>> ---
> >>>>    drivers/mfd/bcm2835-pm.c       | 18 ++++++++++++++++++
> >>>>    include/linux/mfd/bcm2835-pm.h |  1 +
> >>>>    2 files changed, 19 insertions(+)
> >>>>
> >>>> diff --git a/drivers/mfd/bcm2835-pm.c b/drivers/mfd/bcm2835-pm.c
> >>>> index 1656d786993a..da110767c6a4 100644
> >>>> --- a/drivers/mfd/bcm2835-pm.c
> >>>> +++ b/drivers/mfd/bcm2835-pm.c
> >>>> @@ -28,6 +28,8 @@ static const struct mfd_cell bcm2835_power_devs[] = {
> >>>>    static int bcm2835_pm_get_pdata(struct platform_device *pdev,
> >>>>                                   struct bcm2835_pm *pm)
> >>>>    {
> >>>> +       bool is_bcm2711 = of_device_is_compatible(pm->dev->of_node, "brcm,bcm2711-pm");
> >>>> +
> >>>>           /* If no 'reg-names' property is found we can assume we're using old
> >>>>            * firmware.
> >>>>            */
> >>>> @@ -39,6 +41,7 @@ static int bcm2835_pm_get_pdata(struct platform_device *pdev,
> >>>>                           return PTR_ERR(pm->base);
> >>>>
> >>>>                   pm->asb = devm_platform_ioremap_resource(pdev, 1);
> >>>> +               pm->rpivid_asb = devm_platform_ioremap_resource(pdev, 2);
> >>> Shouldn't we check if is_bcm2711 before we assign rpivid_asb above?
> >> Yes, make sense.
> >>>>           } else {
> >>>>                   struct resource *res;
> >>>>
> >>>> @@ -50,11 +53,25 @@ static int bcm2835_pm_get_pdata(struct platform_device *pdev,
> >>>>                                                       "asb");
> >>>>                   if (res)
> >>>>                           pm->asb = devm_ioremap_resource(&pdev->dev, res);
> >>>> +
> >>>> +               res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> >>>> +                                                   "rpivid_asb");
> >>>> +               if (res)
> >>>> +                       pm->rpivid_asb = devm_ioremap_resource(&pdev->dev,
> >>>> +                                                               res);
> >>>>           }
> >>>>
> >>>>           if (IS_ERR(pm->asb))
> >>>>                   pm->asb = NULL;
> >>>>
> >>>> +       if (IS_ERR(pm->rpivid_asb))
> >>>> +               pm->rpivid_asb = NULL;
> >>>> +
> >>>> +       if (pm->rpivid_asb && !is_bcm2711) {
> >>>> +               dev_err(pm->dev, "RPiVid ASB support only present in BCM2711\n");
> >>> Should we ever get into this situation? If it's an older get RPi I'm
> >>> guessing pm->rpivid_asb should have been set to NULL from the error
> >>> above.
> >> I think this was a warning for older BCM2711 downstream DT files which
> >> had rpivid_asb register but no BCM2711 compatible.
> > Based on the history I remember that makes sense, but the warning is a
> > bit misleading in the context given it would actually be a 2711. Is it
> > purely a DT check or is does the version of VC firmware come into play
> > here too, I seem to remember firmware was used to make some things
> > work on older OSes in the early days of RPi4 (although it's hard to
> > tell exactly what was done, and I've tried to forget).
>
> Sorry, my fault. This isn't a warning. It's an error. This driver bail
> out as soon the DT is unexpected, which could otherwise lead to
> unexpected behavior of the power driver (rpivid_asb will be interpret as
> BCM2711). There is no firmware involved, just a pure DT. Maybe i should
> add a comment about this assumption here.
>
> Suggested error message:
>
> "Unexpected rpivid_asb register, please fix your DTB.\ņ"

Maybe "please fix/update your DTB" but either way is good for me.

> >
> >>>> +               return -EINVAL;
> >>>> +       }
> >>>> +
> >>>>           return 0;
> >>>>    }
> >>>>
> >>>> @@ -95,6 +112,7 @@ static int bcm2835_pm_probe(struct platform_device *pdev)
> >>>>    static const struct of_device_id bcm2835_pm_of_match[] = {
> >>>>           { .compatible = "brcm,bcm2835-pm-wdt", },
> >>>>           { .compatible = "brcm,bcm2835-pm", },
> >>>> +       { .compatible = "brcm,bcm2711-pm", },
> >>>>           {},
> >>>>    };
> >>>>    MODULE_DEVICE_TABLE(of, bcm2835_pm_of_match);
> >>>> diff --git a/include/linux/mfd/bcm2835-pm.h b/include/linux/mfd/bcm2835-pm.h
> >>>> index ed37dc40e82a..f70a810c55f7 100644
> >>>> --- a/include/linux/mfd/bcm2835-pm.h
> >>>> +++ b/include/linux/mfd/bcm2835-pm.h
> >>>> @@ -9,6 +9,7 @@ struct bcm2835_pm {
> >>>>           struct device *dev;
> >>>>           void __iomem *base;
> >>>>           void __iomem *asb;
> >>>> +       void __iomem *rpivid_asb;
> >>>>    };
> >>>>
> >>>>    #endif /* BCM2835_MFD_PM_H */
> >>>> --
> >>>> 2.25.1
> >>>>
> >>> _______________________________________________
> >>> linux-arm-kernel mailing list
> >>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux