| Message ID | 20210611140124.v2.7.I2049e180dca12e0d1b3178bfc7292dcf9e05ac28@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor MTK MDP driver into core/components | expand |
Hi, On Fri, Jun 11, 2021 at 12:05 PM Eizan Miyamoto <eizan@chromium.org> wrote: > > ... Instead of depending on the presence of a mediatek,vpu property in > the device node. > > This property was originally added to link to the vpu node so that the > mtk_mdp_core driver could pass the right device to > vpu_wdt_reg_handler(). However in a previous patch in this series, > the driver has been modified to search the device tree for that node > instead. > > This property was also used to indicate the primary MDP device, so that > it can be passed to the V4L2 subsystem as well as register it to be > used when setting up queues in the open() callback for the filesystem > device node that is created. In this case, assuming that the primary > MDP device is the one with a specific name at a specific address seems > useable because the alternative is to add a property to the device tree > which doesn't actually represent any facet of hardware (i.e., this > being the primary MDP device is a software decision). In other words, > this solution is equally as arbitrary, but doesn't add a property to > a device node which is unrelated to the hardware present. > > Signed-off-by: Eizan Miyamoto <eizan@chromium.org> > --- > > (no changes since v1) > > drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 15 ++++----------- > drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 6 ++++++ > 2 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > index 59911ee063c1..77ddc18c90e2 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > @@ -146,17 +146,11 @@ static int mtk_mdp_comp_bind(struct device *dev, struct device *master, > { > struct mtk_mdp_comp *comp = dev_get_drvdata(dev); > struct mtk_mdp_dev *mdp = data; > - struct device_node *vpu_node; > > mtk_mdp_register_component(mdp, comp); > > - /* > - * If this component has a "mediatek-vpu" property, it is responsible for > - * notifying the mdp master driver about it so it can be further initialized > - * later. > - */ > - vpu_node = of_parse_phandle(dev->of_node, "mediatek,vpu", 0); > - if (vpu_node) { > + if (strcmp(dev->of_node->full_name, "rdma@14001000") == 0) { > + /* This is the primary MDP device node. */ This seems quite fragile as it likely doesn't hold true across different SoCs. You would then end up with a bunch of of_device_is_compatible() and name checking combinations. Would it make more sense to check against DT aliases? Since this device already has the mdp-rdma0 alias associated with it, we could mandate some alias naming scheme going forward? ChenYu > int ret; > > ret = v4l2_device_register(dev, &mdp->v4l2_dev); > @@ -172,9 +166,8 @@ static int mtk_mdp_comp_bind(struct device *dev, struct device *master, > } > > /* > - * presence of the "mediatek,vpu" property in a device node > - * indicates that it is the primary MDP rdma device and MDP DMA > - * ops should be handled by its DMA callbacks. > + * MDP DMA ops will be handled by the DMA callbacks associated with this > + * device; > */ > mdp->rdma_dev = dev; > } > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > index 523e3c2798e1..be7d35b3e3ff 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > @@ -157,6 +157,12 @@ static int mtk_mdp_master_bind(struct device *dev) > goto err_component_bind_all; > } > > + if (mdp->rdma_dev == NULL) { > + dev_err(dev, "Primary MDP device not found"); > + status = -ENODEV; > + goto err_component_bind_all; > + } > + > vpu_node = of_find_node_by_name(NULL, "vpu"); > if (!vpu_node) { > dev_err(dev, "unable to finddrivers/soc/mediatek/mtk-mmsys.c vpu node"); > -- > 2.32.0.272.g935e593368-goog > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c index 59911ee063c1..77ddc18c90e2 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c @@ -146,17 +146,11 @@ static int mtk_mdp_comp_bind(struct device *dev, struct device *master, { struct mtk_mdp_comp *comp = dev_get_drvdata(dev); struct mtk_mdp_dev *mdp = data; - struct device_node *vpu_node; mtk_mdp_register_component(mdp, comp); - /* - * If this component has a "mediatek-vpu" property, it is responsible for - * notifying the mdp master driver about it so it can be further initialized - * later. - */ - vpu_node = of_parse_phandle(dev->of_node, "mediatek,vpu", 0); - if (vpu_node) { + if (strcmp(dev->of_node->full_name, "rdma@14001000") == 0) { + /* This is the primary MDP device node. */ int ret; ret = v4l2_device_register(dev, &mdp->v4l2_dev); @@ -172,9 +166,8 @@ static int mtk_mdp_comp_bind(struct device *dev, struct device *master, } /* - * presence of the "mediatek,vpu" property in a device node - * indicates that it is the primary MDP rdma device and MDP DMA - * ops should be handled by its DMA callbacks. + * MDP DMA ops will be handled by the DMA callbacks associated with this + * device; */ mdp->rdma_dev = dev; } diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c index 523e3c2798e1..be7d35b3e3ff 100644 --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c @@ -157,6 +157,12 @@ static int mtk_mdp_master_bind(struct device *dev) goto err_component_bind_all; } + if (mdp->rdma_dev == NULL) { + dev_err(dev, "Primary MDP device not found"); + status = -ENODEV; + goto err_component_bind_all; + } + vpu_node = of_find_node_by_name(NULL, "vpu"); if (!vpu_node) { dev_err(dev, "unable to find vpu node");
... Instead of depending on the presence of a mediatek,vpu property in the device node. This property was originally added to link to the vpu node so that the mtk_mdp_core driver could pass the right device to vpu_wdt_reg_handler(). However in a previous patch in this series, the driver has been modified to search the device tree for that node instead. This property was also used to indicate the primary MDP device, so that it can be passed to the V4L2 subsystem as well as register it to be used when setting up queues in the open() callback for the filesystem device node that is created. In this case, assuming that the primary MDP device is the one with a specific name at a specific address seems useable because the alternative is to add a property to the device tree which doesn't actually represent any facet of hardware (i.e., this being the primary MDP device is a software decision). In other words, this solution is equally as arbitrary, but doesn't add a property to a device node which is unrelated to the hardware present. Signed-off-by: Eizan Miyamoto <eizan@chromium.org> --- (no changes since v1) drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 15 ++++----------- drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 6 ++++++ 2 files changed, 10 insertions(+), 11 deletions(-)