| Message ID | 20190722081243.2084226-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: media/davinci_vpfe: fix pinmux setup compilation | expand |
Hi Arnd, On 22/07/19 1:42 PM, Arnd Bergmann wrote: > The dm365_isif staging driver uses an odd method for configuring its > pin muxing by calling directly into low-level davinci platform specific > code, even when being compile-tested for other platforms. > > As we want davinci to be part of a multi-platform kernel, this will > cause a build failure when those headers are no longer exported even > for davinci: > > drivers/staging/media/davinci_vpfe/dm365_isif.c: In function 'vpfe_isif_init': > drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:2: error: implicit declaration of function 'davinci_cfg_reg'; did you mean 'omap_cfg_reg'? [-Werror=implicit-function-declaration] > davinci_cfg_reg(DM365_VIN_CAM_WEN); > ^~~~~~~~~~~~~~~ > omap_cfg_reg > drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error: 'DM365_VIN_CAM_WEN' undeclared (first use in this function); did you mean 'DM365_ISIF_MAX_CLDC'? > davinci_cfg_reg(DM365_VIN_CAM_WEN); > ^~~~~~~~~~~~~~~~~ > > Digging further, it seems that the platform data structures defined > in drivers/staging/media/davinci_vpfe/vpfe.h are an incompatible > version of the same structures in include/media/davinci/vpfe_capture.h, > which is the version that is used by the platform code, so the > combination that exists in the mainline kernel cannot be used. > > The platform code already has an abstraction for the pinmux, > in the form of the dm365_isif_setup_pinmux() helper. If we want > to ever get to use the staging driver again, this needs to be > read from the platform data passed to this driver, or rewritten > to use the pinmux framework. > > For the moment, pretend we pass the helper function in the > staging platform driver to get it to build cleanly. I could > not figure out how the staging driver relates to the code > in drivers/media/platform/davinci/, some clarification on that > would be helpful to decide what the long-term plan on this > should be to either remove the staging driver as obsolete or > integrate it with the rest in a way that actually works. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I looked at the history of updates on this driver over last 4 years. None of them are towards fixing some issue found with the driver during actual usage or for improving its design to move it out of staging. I think no one is really using it or working on moving it out of staging. Perhaps the right thing to do would be to delete it. Thanks, Sekhar
diff --git a/drivers/staging/media/davinci_vpfe/Makefile b/drivers/staging/media/davinci_vpfe/Makefile index 0ae8c5014f74..3b93e0583940 100644 --- a/drivers/staging/media/davinci_vpfe/Makefile +++ b/drivers/staging/media/davinci_vpfe/Makefile @@ -4,8 +4,3 @@ obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci-vfpe.o davinci-vfpe-objs := \ dm365_isif.o dm365_ipipe_hw.o dm365_ipipe.o \ dm365_resizer.o dm365_ipipeif.o vpfe_mc_capture.o vpfe_video.o - -# Allow building it with COMPILE_TEST on other archs -ifndef CONFIG_ARCH_DAVINCI -ccflags-y += -I $(srctree)/arch/arm/mach-davinci/include/ -endif diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c b/drivers/staging/media/davinci_vpfe/dm365_isif.c index 05a997f7aa5d..5cfd52ea3ba7 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c @@ -17,6 +17,7 @@ */ #include <linux/delay.h> +#include "vpfe.h" #include "dm365_isif.h" #include "vpfe_mc_capture.h" @@ -1983,6 +1984,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct platform_device *pdev) struct v4l2_subdev *sd = &isif->subdev; struct media_pad *pads = &isif->pads[0]; struct media_entity *me = &sd->entity; + struct vpfe_config *cfg = pdev->dev.platform_data; static resource_size_t res_len; struct resource *res; void __iomem *addr; @@ -2023,11 +2025,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct platform_device *pdev) } i++; } - davinci_cfg_reg(DM365_VIN_CAM_WEN); - davinci_cfg_reg(DM365_VIN_CAM_VD); - davinci_cfg_reg(DM365_VIN_CAM_HD); - davinci_cfg_reg(DM365_VIN_YIN4_7_EN); - davinci_cfg_reg(DM365_VIN_YIN0_3_EN); + cfg->isif_setup_pinmux(); /* queue ops */ isif->video_out.ops = &isif_video_ops; diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.h b/drivers/staging/media/davinci_vpfe/dm365_isif.h index 0e1fe472fb2b..82eeba9c24c2 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif.h +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.h @@ -21,8 +21,6 @@ #include <linux/platform_device.h> -#include <mach/mux.h> - #include <media/davinci/vpfe_types.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> diff --git a/drivers/staging/media/davinci_vpfe/vpfe.h b/drivers/staging/media/davinci_vpfe/vpfe.h index 1f8e011fc162..54ef6720ceeb 100644 --- a/drivers/staging/media/davinci_vpfe/vpfe.h +++ b/drivers/staging/media/davinci_vpfe/vpfe.h @@ -74,6 +74,8 @@ struct vpfe_config { char *card_name; /* setup function for the input path */ int (*setup_input)(enum vpfe_subdev_id id); + /* point to dm365_isif_setup_pinmux() */ + void (*isif_setup_pinmux)(void); /* number of clocks */ int num_clocks; /* clocks used for vpfe capture */
The dm365_isif staging driver uses an odd method for configuring its pin muxing by calling directly into low-level davinci platform specific code, even when being compile-tested for other platforms. As we want davinci to be part of a multi-platform kernel, this will cause a build failure when those headers are no longer exported even for davinci: drivers/staging/media/davinci_vpfe/dm365_isif.c: In function 'vpfe_isif_init': drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:2: error: implicit declaration of function 'davinci_cfg_reg'; did you mean 'omap_cfg_reg'? [-Werror=implicit-function-declaration] davinci_cfg_reg(DM365_VIN_CAM_WEN); ^~~~~~~~~~~~~~~ omap_cfg_reg drivers/staging/media/davinci_vpfe/dm365_isif.c:2031:18: error: 'DM365_VIN_CAM_WEN' undeclared (first use in this function); did you mean 'DM365_ISIF_MAX_CLDC'? davinci_cfg_reg(DM365_VIN_CAM_WEN); ^~~~~~~~~~~~~~~~~ Digging further, it seems that the platform data structures defined in drivers/staging/media/davinci_vpfe/vpfe.h are an incompatible version of the same structures in include/media/davinci/vpfe_capture.h, which is the version that is used by the platform code, so the combination that exists in the mainline kernel cannot be used. The platform code already has an abstraction for the pinmux, in the form of the dm365_isif_setup_pinmux() helper. If we want to ever get to use the staging driver again, this needs to be read from the platform data passed to this driver, or rewritten to use the pinmux framework. For the moment, pretend we pass the helper function in the staging platform driver to get it to build cleanly. I could not figure out how the staging driver relates to the code in drivers/media/platform/davinci/, some clarification on that would be helpful to decide what the long-term plan on this should be to either remove the staging driver as obsolete or integrate it with the rest in a way that actually works. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/staging/media/davinci_vpfe/Makefile | 5 ----- drivers/staging/media/davinci_vpfe/dm365_isif.c | 8 +++----- drivers/staging/media/davinci_vpfe/dm365_isif.h | 2 -- drivers/staging/media/davinci_vpfe/vpfe.h | 2 ++ 4 files changed, 5 insertions(+), 12 deletions(-)