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? The decision not to use bits makes sense I believe. [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? > } 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. > + 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 >