| Message ID | 20240526195826.109008-2-sui.jingfeng@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/loongson: Introduce component framework support | expand |
On Mon, May 27, 2024 at 03:58:24AM +0800, Sui Jingfeng wrote: > In some display subsystems, the functionality of a PCIe device may too > complex to be managed by a single driver. A split of the functionality > into child devices can helpful to achieve better modularity. > > Another benefit is that we could migrate the dependency on exterinal > modules to a submodule level with the helper created. For example, it's > not uncommon that some exterinal module will return -EPROBE_DEFER to our > driver during probe time. KMS driver has to tear down everything when it > receives -EPROBE_DEFER, the problem is that it's completely not necessary > and rising cyclic dependency problems if not process correctly. > > Add the loongson_create_platform_device() function, which allows the KMS > driver to create sub-devices for it. The manually created decice acts as > agents for the principal part, migrate the potential issue to submodule. > > Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev> > --- > drivers/gpu/drm/loongson/loongson_device.c | 42 ++++++++++++++++++++++ > drivers/gpu/drm/loongson/lsdc_drv.h | 6 ++++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/gpu/drm/loongson/loongson_device.c b/drivers/gpu/drm/loongson/loongson_device.c > index 9986c8a2a255..b268549d643e 100644 > --- a/drivers/gpu/drm/loongson/loongson_device.c > +++ b/drivers/gpu/drm/loongson/loongson_device.c > @@ -4,6 +4,7 @@ > */ > > #include <linux/pci.h> > +#include <linux/platform_device.h> > > #include "lsdc_drv.h" > > @@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id) > { > return __chip_id_desc_table[chip_id]; > } > + > +int loongson_create_platform_device(struct device *parent, > + const char *name, int id, > + struct resource *pres, > + void *data, > + struct platform_device **ppdev) > +{ > + struct platform_device *pdev; > + int ret; > + > + pdev = platform_device_alloc(name, id); > + if (!pdev) > + return -ENOMEM; > + > + pdev->dev.parent = parent; > + > + if (pres) { > + ret = platform_device_add_resources(pdev, pres, 1); > + if (ret) { > + platform_device_put(pdev); > + return ret; > + } > + } > + > + if (data) { > + void *pdata = kmalloc(sizeof(void *), GFP_KERNEL); > + > + *(void **)pdata = data; > + pdev->dev.platform_data = pdata; > + } > + > + ret = platform_device_add(pdev); > + if (ret) { > + platform_device_put(pdev); > + return ret; > + } Please use platform_device_register_resndata(). > + > + *ppdev = pdev; > + > + return 0; > +} > diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h > index fbf2d760ef27..a2c6b496a69f 100644 > --- a/drivers/gpu/drm/loongson/lsdc_drv.h > +++ b/drivers/gpu/drm/loongson/lsdc_drv.h > @@ -47,6 +47,12 @@ enum loongson_chip_id { > const struct lsdc_desc * > lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip); > > +int loongson_create_platform_device(struct device *parent, > + const char *name, int id, > + struct resource *pres, > + void *data, > + struct platform_device **ppdev); > + > struct lsdc_kms_funcs; > > /* DC specific */ > -- > 2.34.1 >
> In some display subsystems, the functionality of a PCIe device may too might be? … > into child devices can helpful to … be? > Another benefit is that we could migrate the dependency on exterinal external? … > and rising cyclic dependency problems if not process correctly. processed? … > driver to create sub-devices for it. The manually created decice acts as device? > agents for the principal part, migrate the potential issue to submodule. an agent? Please improve your change descriptions considerably. … > +++ b/drivers/gpu/drm/loongson/loongson_device.c … > @@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id) > { > return __chip_id_desc_table[chip_id]; > } > + > +int loongson_create_platform_device(struct device *parent, > + const char *name, int id, > + struct resource *pres, > + void *data, > + struct platform_device **ppdev) > +{ … > + ret = platform_device_add_resources(pdev, pres, 1); > + if (ret) { > + platform_device_put(pdev); > + return ret; > + } … > + ret = platform_device_add(pdev); > + if (ret) { > + platform_device_put(pdev); > + return ret; > + } … Please use a goto chain for common exception handling. https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources Regards, Markus
diff --git a/drivers/gpu/drm/loongson/loongson_device.c b/drivers/gpu/drm/loongson/loongson_device.c index 9986c8a2a255..b268549d643e 100644 --- a/drivers/gpu/drm/loongson/loongson_device.c +++ b/drivers/gpu/drm/loongson/loongson_device.c @@ -4,6 +4,7 @@ */ #include <linux/pci.h> +#include <linux/platform_device.h> #include "lsdc_drv.h" @@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id) { return __chip_id_desc_table[chip_id]; } + +int loongson_create_platform_device(struct device *parent, + const char *name, int id, + struct resource *pres, + void *data, + struct platform_device **ppdev) +{ + struct platform_device *pdev; + int ret; + + pdev = platform_device_alloc(name, id); + if (!pdev) + return -ENOMEM; + + pdev->dev.parent = parent; + + if (pres) { + ret = platform_device_add_resources(pdev, pres, 1); + if (ret) { + platform_device_put(pdev); + return ret; + } + } + + if (data) { + void *pdata = kmalloc(sizeof(void *), GFP_KERNEL); + + *(void **)pdata = data; + pdev->dev.platform_data = pdata; + } + + ret = platform_device_add(pdev); + if (ret) { + platform_device_put(pdev); + return ret; + } + + *ppdev = pdev; + + return 0; +} diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h index fbf2d760ef27..a2c6b496a69f 100644 --- a/drivers/gpu/drm/loongson/lsdc_drv.h +++ b/drivers/gpu/drm/loongson/lsdc_drv.h @@ -47,6 +47,12 @@ enum loongson_chip_id { const struct lsdc_desc * lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip); +int loongson_create_platform_device(struct device *parent, + const char *name, int id, + struct resource *pres, + void *data, + struct platform_device **ppdev); + struct lsdc_kms_funcs; /* DC specific */
In some display subsystems, the functionality of a PCIe device may too complex to be managed by a single driver. A split of the functionality into child devices can helpful to achieve better modularity. Another benefit is that we could migrate the dependency on exterinal modules to a submodule level with the helper created. For example, it's not uncommon that some exterinal module will return -EPROBE_DEFER to our driver during probe time. KMS driver has to tear down everything when it receives -EPROBE_DEFER, the problem is that it's completely not necessary and rising cyclic dependency problems if not process correctly. Add the loongson_create_platform_device() function, which allows the KMS driver to create sub-devices for it. The manually created decice acts as agents for the principal part, migrate the potential issue to submodule. Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev> --- drivers/gpu/drm/loongson/loongson_device.c | 42 ++++++++++++++++++++++ drivers/gpu/drm/loongson/lsdc_drv.h | 6 ++++ 2 files changed, 48 insertions(+)