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 <[email protected]>
---
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(-)
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 */
--
2.20.0
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 <[email protected]>
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