On Fri, Nov 17, 2023 at 03:06:35PM +0800, Andy Yan wrote: > Hi Sebastian: > > On 11/16/23 21:47, Sebastian Reichel wrote: > > Hi, > > > > On Thu, Nov 16, 2023 at 06:39:40PM +0800, Andy Yan wrote: > > > > > vop2->sys_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf"); > > > > This already lacks an error check, shame on me... > > > > > > > > > + vop2->vop_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,vop-grf"); > > > > > + vop2->vo1_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,vo1-grf"); > > > > > + vop2->sys_pmu = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,pmu"); > > > > ... but please don't duplicate that. > > > It a little difficult to find a proper way to do the check, as not every soc need all these phandles. > > > > > > Do i need check it per soc? > > I suggest adding a u32 flags to struct vop2_data and then have > > something like this: > > > > if (vop2_data->flags & VOP2_HAS_VOP_GRF) { > > vop2->vop_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,vop-grf"); > > if (IS_ERR(vop2->vop_grf)) > > return dev_err_probe(dev, PTR_ERR(vop2->vop_grf) "cannot get vop-grf"); > > } > > > > if (vop2_data->flags & VOP2_HAS_VO1_GRF) { > > vop2->vo1_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,vo1-grf"); > > if (IS_ERR(vop2->vo1_grf)) > > return dev_err_probe(dev, PTR_ERR(vop2->vo1_grf) "cannot get vo1-grf"); > > } > > > > ... > > > I can do it like this if Sascha is also happy with it. Yes, I am. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |