[PATCH 07/11] mfd: bcm2835-pm: Add support for BCM2711
Stefan Wahren
stefan.wahren at i2se.com
Tue May 31 15:03:52 PDT 2022
Hi Peter,
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 at i2se.com> 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 at i2se.com> 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@gmail.com/
>>>
>>>> 'reg-names.'
>>>>
>>>> Signed-off-by: Stefan Wahren <stefan.wahren at i2se.com>
>>>> ---
>>>> 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.\ņ"
>
>>>> + 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 at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list