| Message ID | f618981fec34acc5eee211b34a0018752634af9c.1522949748.git.mchehab@s-opensource.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mauro, Thank you for the patch. On Thursday, 5 April 2018 20:54:02 EEST Mauro Carvalho Chehab wrote: > There aren't much things required for it to build with COMPILE_TEST. > It just needs to provide stub for an arm-dependent include. > > Let's replicate the same solution used by ipmmu-vmsa, in order > to allow building omap3 with COMPILE_TEST. > > The actual logic here came from this driver: > > drivers/iommu/ipmmu-vmsa.c > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > --- > drivers/media/platform/Kconfig | 8 ++++---- > drivers/media/platform/omap3isp/isp.c | 7 +++++++ > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index c7a1cf8a1b01..03c9dfeb7781 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -62,12 +62,12 @@ config VIDEO_MUX > > config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > depends on HAS_DMA && OF > - depends on OMAP_IOMMU > - select ARM_DMA_USE_IOMMU > + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST) > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU > select VIDEOBUF2_DMA_CONTIG > - select MFD_SYSCON > + select MFD_SYSCON if ARCH_OMAP3 > select V4L2_FWNODE > ---help--- > Driver for an OMAP 3 camera controller. > diff --git a/drivers/media/platform/omap3isp/isp.c > b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f > 100644 > --- a/drivers/media/platform/omap3isp/isp.c > +++ b/drivers/media/platform/omap3isp/isp.c > @@ -61,7 +61,14 @@ > #include <linux/sched.h> > #include <linux/vmalloc.h> > > +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA) > #include <asm/dma-iommu.h> > +#else > +#define arm_iommu_create_mapping(...) NULL > +#define arm_iommu_attach_device(...) -ENODEV > +#define arm_iommu_release_mapping(...) do {} while (0) > +#define arm_iommu_detach_device(...) do {} while (0) > +#endif I don't think it's the job of a driver to define those stubs, sorry. Otherwise where do you stop ? If you have half of the code that is architecture- dependent, would you stub it ? And what if the stubs you define here generate warnings in static analyzers ? If you want to make drivers compile for all architectures, the APIs they use must be defined in linux/, not in asm/. They can be stubbed there when not implemented in a particular architecture, but not in the driver. > #include <media/v4l2-common.h> > #include <media/v4l2-fwnode.h>
Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > Hi Mauro, > > Thank you for the patch. > > On Thursday, 5 April 2018 20:54:02 EEST Mauro Carvalho Chehab wrote: > > There aren't much things required for it to build with COMPILE_TEST. > > It just needs to provide stub for an arm-dependent include. > > > > Let's replicate the same solution used by ipmmu-vmsa, in order > > to allow building omap3 with COMPILE_TEST. > > > > The actual logic here came from this driver: > > > > drivers/iommu/ipmmu-vmsa.c > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > --- > > drivers/media/platform/Kconfig | 8 ++++---- > > drivers/media/platform/omap3isp/isp.c | 7 +++++++ > > 2 files changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > > index c7a1cf8a1b01..03c9dfeb7781 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -62,12 +62,12 @@ config VIDEO_MUX > > > > config VIDEO_OMAP3 > > tristate "OMAP 3 Camera support" > > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > > depends on HAS_DMA && OF > > - depends on OMAP_IOMMU > > - select ARM_DMA_USE_IOMMU > > + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST) > > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU > > select VIDEOBUF2_DMA_CONTIG > > - select MFD_SYSCON > > + select MFD_SYSCON if ARCH_OMAP3 > > select V4L2_FWNODE > > ---help--- > > Driver for an OMAP 3 camera controller. > > diff --git a/drivers/media/platform/omap3isp/isp.c > > b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f > > 100644 > > --- a/drivers/media/platform/omap3isp/isp.c > > +++ b/drivers/media/platform/omap3isp/isp.c > > @@ -61,7 +61,14 @@ > > #include <linux/sched.h> > > #include <linux/vmalloc.h> > > > > +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA) > > #include <asm/dma-iommu.h> > > +#else > > +#define arm_iommu_create_mapping(...) NULL > > +#define arm_iommu_attach_device(...) -ENODEV > > +#define arm_iommu_release_mapping(...) do {} while (0) > > +#define arm_iommu_detach_device(...) do {} while (0) > > +#endif > > I don't think it's the job of a driver to define those stubs, sorry. Otherwise > where do you stop ? If you have half of the code that is architecture- > dependent, would you stub it ? And what if the stubs you define here generate > warnings in static analyzers ? I agree that we should avoid doing that as a general case, but see below. > If you want to make drivers compile for all architectures, the APIs they use > must be defined in linux/, not in asm/. They can be stubbed there when not > implemented in a particular architecture, but not in the driver. In this specific case, the same approach taken here is already needed by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside drivers/iommu: drivers/iommu/ipmmu-vmsa.c Also, this API is used only by 3 drivers [1]: drivers/iommu/ipmmu-vmsa.c drivers/iommu/mtk_iommu_v1.c drivers/media/platform/omap3isp/isp.c [1] as blamed by git grep -l arm_iommu_create_mapping That hardly seems to be an arch-specific iommu solution, but, instead, some hack used by only three drivers or some legacy iommu binding. The omap3isp is, btw, the only driver outside drivers/iommu that needs it. So, it sounds that other driver uses some other approach, but hardly it would be worth to change this driver to use something else. So, better to stick with the same solution the Renesas driver used. Thanks, Mauro
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-drivers-under-drivers-media-to-build-with-COMPILE_TEST/20180406-164215
base: git://linuxtv.org/media_tree.git master
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
In file included from drivers/media/platform/omap3isp/isp.c:78:0:
drivers/media/platform/omap3isp/isp.h:130:16: error: field 'hw' has incomplete type
struct clk_hw hw;
^~
In file included from include/asm-generic/bug.h:5:0,
from ./arch/openrisc/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from arch/openrisc/include/asm/cacheflush.h:21,
from drivers/media/platform/omap3isp/isp.c:45:
drivers/media/platform/omap3isp/isp.c: In function 'isp_xclk_prepare':
include/linux/kernel.h:938:32: error: dereferencing pointer to incomplete type 'struct clk_hw'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:938:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:938:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:167:26: note: in expansion of macro 'container_of'
#define to_isp_xclk(_hw) container_of(_hw, struct isp_xclk, hw)
^~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:187:26: note: in expansion of macro 'to_isp_xclk'
struct isp_xclk *xclk = to_isp_xclk(hw);
^~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c: At top level:
drivers/media/platform/omap3isp/isp.c:282:21: error: variable 'isp_xclk_ops' has initializer but incomplete type
static const struct clk_ops isp_xclk_ops = {
^~~~~~~
>> drivers/media/platform/omap3isp/isp.c:283:2: error: unknown field 'prepare' specified in initializer
.prepare = isp_xclk_prepare,
^
drivers/media/platform/omap3isp/isp.c:283:13: warning: excess elements in struct initializer
.prepare = isp_xclk_prepare,
^~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:283:13: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:284:2: error: unknown field 'unprepare' specified in initializer
.unprepare = isp_xclk_unprepare,
^
drivers/media/platform/omap3isp/isp.c:284:15: warning: excess elements in struct initializer
.unprepare = isp_xclk_unprepare,
^~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:284:15: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:285:2: error: unknown field 'enable' specified in initializer
.enable = isp_xclk_enable,
^
drivers/media/platform/omap3isp/isp.c:285:12: warning: excess elements in struct initializer
.enable = isp_xclk_enable,
^~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:285:12: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:286:2: error: unknown field 'disable' specified in initializer
.disable = isp_xclk_disable,
^
drivers/media/platform/omap3isp/isp.c:286:13: warning: excess elements in struct initializer
.disable = isp_xclk_disable,
^~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:286:13: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:287:2: error: unknown field 'recalc_rate' specified in initializer
.recalc_rate = isp_xclk_recalc_rate,
^
drivers/media/platform/omap3isp/isp.c:287:17: warning: excess elements in struct initializer
.recalc_rate = isp_xclk_recalc_rate,
^~~~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:287:17: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:288:2: error: unknown field 'round_rate' specified in initializer
.round_rate = isp_xclk_round_rate,
^
drivers/media/platform/omap3isp/isp.c:288:16: warning: excess elements in struct initializer
.round_rate = isp_xclk_round_rate,
^~~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:288:16: note: (near initialization for 'isp_xclk_ops')
>> drivers/media/platform/omap3isp/isp.c:289:2: error: unknown field 'set_rate' specified in initializer
.set_rate = isp_xclk_set_rate,
^
drivers/media/platform/omap3isp/isp.c:289:14: warning: excess elements in struct initializer
.set_rate = isp_xclk_set_rate,
^~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:289:14: note: (near initialization for 'isp_xclk_ops')
drivers/media/platform/omap3isp/isp.c:294:21: error: variable 'isp_xclk_init_data' has initializer but incomplete type
static const struct clk_init_data isp_xclk_init_data = {
^~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:295:2: error: unknown field 'name' specified in initializer
.name = "cam_xclk",
^
drivers/media/platform/omap3isp/isp.c:295:10: warning: excess elements in struct initializer
.name = "cam_xclk",
^~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:295:10: note: (near initialization for 'isp_xclk_init_data')
drivers/media/platform/omap3isp/isp.c:296:2: error: unknown field 'ops' specified in initializer
.ops = &isp_xclk_ops,
^
drivers/media/platform/omap3isp/isp.c:296:9: warning: excess elements in struct initializer
.ops = &isp_xclk_ops,
^
drivers/media/platform/omap3isp/isp.c:296:9: note: (near initialization for 'isp_xclk_init_data')
drivers/media/platform/omap3isp/isp.c:297:2: error: unknown field 'parent_names' specified in initializer
.parent_names = &isp_xclk_parent_name,
^
drivers/media/platform/omap3isp/isp.c:297:18: warning: excess elements in struct initializer
.parent_names = &isp_xclk_parent_name,
^
drivers/media/platform/omap3isp/isp.c:297:18: note: (near initialization for 'isp_xclk_init_data')
drivers/media/platform/omap3isp/isp.c:298:2: error: unknown field 'num_parents' specified in initializer
.num_parents = 1,
^
drivers/media/platform/omap3isp/isp.c:298:17: warning: excess elements in struct initializer
.num_parents = 1,
^
drivers/media/platform/omap3isp/isp.c:298:17: note: (near initialization for 'isp_xclk_init_data')
drivers/media/platform/omap3isp/isp.c: In function 'isp_xclk_init':
drivers/media/platform/omap3isp/isp.c:315:23: error: storage size of 'init' isn't known
struct clk_init_data init;
^~~~
>> drivers/media/platform/omap3isp/isp.c:341:15: error: implicit declaration of function 'clk_register' [-Werror=implicit-function-declaration]
xclk->clk = clk_register(NULL, &xclk->hw);
^~~~~~~~~~~~
>> drivers/media/platform/omap3isp/isp.c:347:3: error: implicit declaration of function 'of_clk_add_provider' [-Werror=implicit-function-declaration]
of_clk_add_provider(np, isp_xclk_src_get, isp);
^~~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:315:23: warning: unused variable 'init' [-Wunused-variable]
struct clk_init_data init;
^~~~
drivers/media/platform/omap3isp/isp.c: In function 'isp_xclk_cleanup':
>> drivers/media/platform/omap3isp/isp.c:358:3: error: implicit declaration of function 'of_clk_del_provider' [-Werror=implicit-function-declaration]
of_clk_del_provider(np);
^~~~~~~~~~~~~~~~~~~
>> drivers/media/platform/omap3isp/isp.c:364:4: error: implicit declaration of function 'clk_unregister' [-Werror=implicit-function-declaration]
clk_unregister(xclk->clk);
^~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c: At top level:
drivers/media/platform/omap3isp/isp.c:282:29: error: storage size of 'isp_xclk_ops' isn't known
static const struct clk_ops isp_xclk_ops = {
^~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:294:35: error: storage size of 'isp_xclk_init_data' isn't known
static const struct clk_init_data isp_xclk_init_data = {
^~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:1020:13: warning: 'isp_resume_modules' defined but not used [-Wunused-function]
static void isp_resume_modules(struct isp_device *isp)
^~~~~~~~~~~~~~~~~~
drivers/media/platform/omap3isp/isp.c:986:12: warning: 'isp_suspend_modules' defined but not used [-Wunused-function]
static int isp_suspend_modules(struct isp_device *isp)
^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/prepare +283 drivers/media/platform/omap3isp/isp.c
9b28ee3c Laurent Pinchart 2012-10-22 184
9b28ee3c Laurent Pinchart 2012-10-22 185 static int isp_xclk_prepare(struct clk_hw *hw)
9b28ee3c Laurent Pinchart 2012-10-22 186 {
9b28ee3c Laurent Pinchart 2012-10-22 @187 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 188
9b28ee3c Laurent Pinchart 2012-10-22 189 omap3isp_get(xclk->isp);
9b28ee3c Laurent Pinchart 2012-10-22 190
9b28ee3c Laurent Pinchart 2012-10-22 191 return 0;
9b28ee3c Laurent Pinchart 2012-10-22 192 }
9b28ee3c Laurent Pinchart 2012-10-22 193
9b28ee3c Laurent Pinchart 2012-10-22 194 static void isp_xclk_unprepare(struct clk_hw *hw)
9b28ee3c Laurent Pinchart 2012-10-22 195 {
9b28ee3c Laurent Pinchart 2012-10-22 196 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 197
9b28ee3c Laurent Pinchart 2012-10-22 198 omap3isp_put(xclk->isp);
9b28ee3c Laurent Pinchart 2012-10-22 199 }
9b28ee3c Laurent Pinchart 2012-10-22 200
9b28ee3c Laurent Pinchart 2012-10-22 201 static int isp_xclk_enable(struct clk_hw *hw)
9b28ee3c Laurent Pinchart 2012-10-22 202 {
9b28ee3c Laurent Pinchart 2012-10-22 203 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 204 unsigned long flags;
9b28ee3c Laurent Pinchart 2012-10-22 205
9b28ee3c Laurent Pinchart 2012-10-22 206 spin_lock_irqsave(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 207 isp_xclk_update(xclk, xclk->divider);
9b28ee3c Laurent Pinchart 2012-10-22 208 xclk->enabled = true;
9b28ee3c Laurent Pinchart 2012-10-22 209 spin_unlock_irqrestore(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 210
9b28ee3c Laurent Pinchart 2012-10-22 211 return 0;
9b28ee3c Laurent Pinchart 2012-10-22 212 }
9b28ee3c Laurent Pinchart 2012-10-22 213
9b28ee3c Laurent Pinchart 2012-10-22 214 static void isp_xclk_disable(struct clk_hw *hw)
9b28ee3c Laurent Pinchart 2012-10-22 215 {
9b28ee3c Laurent Pinchart 2012-10-22 216 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 217 unsigned long flags;
9b28ee3c Laurent Pinchart 2012-10-22 218
9b28ee3c Laurent Pinchart 2012-10-22 219 spin_lock_irqsave(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 220 isp_xclk_update(xclk, 0);
9b28ee3c Laurent Pinchart 2012-10-22 221 xclk->enabled = false;
9b28ee3c Laurent Pinchart 2012-10-22 222 spin_unlock_irqrestore(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 223 }
9b28ee3c Laurent Pinchart 2012-10-22 224
9b28ee3c Laurent Pinchart 2012-10-22 225 static unsigned long isp_xclk_recalc_rate(struct clk_hw *hw,
9b28ee3c Laurent Pinchart 2012-10-22 226 unsigned long parent_rate)
9b28ee3c Laurent Pinchart 2012-10-22 227 {
9b28ee3c Laurent Pinchart 2012-10-22 228 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 229
9b28ee3c Laurent Pinchart 2012-10-22 230 return parent_rate / xclk->divider;
9b28ee3c Laurent Pinchart 2012-10-22 231 }
9b28ee3c Laurent Pinchart 2012-10-22 232
9b28ee3c Laurent Pinchart 2012-10-22 233 static u32 isp_xclk_calc_divider(unsigned long *rate, unsigned long parent_rate)
9b28ee3c Laurent Pinchart 2012-10-22 234 {
9b28ee3c Laurent Pinchart 2012-10-22 235 u32 divider;
9b28ee3c Laurent Pinchart 2012-10-22 236
9b28ee3c Laurent Pinchart 2012-10-22 237 if (*rate >= parent_rate) {
9b28ee3c Laurent Pinchart 2012-10-22 238 *rate = parent_rate;
9b28ee3c Laurent Pinchart 2012-10-22 239 return ISPTCTRL_CTRL_DIV_BYPASS;
9b28ee3c Laurent Pinchart 2012-10-22 240 }
9b28ee3c Laurent Pinchart 2012-10-22 241
aadec012 Laurent Pinchart 2014-09-25 242 if (*rate == 0)
aadec012 Laurent Pinchart 2014-09-25 243 *rate = 1;
aadec012 Laurent Pinchart 2014-09-25 244
9b28ee3c Laurent Pinchart 2012-10-22 245 divider = DIV_ROUND_CLOSEST(parent_rate, *rate);
9b28ee3c Laurent Pinchart 2012-10-22 246 if (divider >= ISPTCTRL_CTRL_DIV_BYPASS)
9b28ee3c Laurent Pinchart 2012-10-22 247 divider = ISPTCTRL_CTRL_DIV_BYPASS - 1;
9b28ee3c Laurent Pinchart 2012-10-22 248
9b28ee3c Laurent Pinchart 2012-10-22 249 *rate = parent_rate / divider;
9b28ee3c Laurent Pinchart 2012-10-22 250 return divider;
9b28ee3c Laurent Pinchart 2012-10-22 251 }
9b28ee3c Laurent Pinchart 2012-10-22 252
9b28ee3c Laurent Pinchart 2012-10-22 253 static long isp_xclk_round_rate(struct clk_hw *hw, unsigned long rate,
9b28ee3c Laurent Pinchart 2012-10-22 254 unsigned long *parent_rate)
9b28ee3c Laurent Pinchart 2012-10-22 255 {
9b28ee3c Laurent Pinchart 2012-10-22 256 isp_xclk_calc_divider(&rate, *parent_rate);
9b28ee3c Laurent Pinchart 2012-10-22 257 return rate;
9b28ee3c Laurent Pinchart 2012-10-22 258 }
9b28ee3c Laurent Pinchart 2012-10-22 259
9b28ee3c Laurent Pinchart 2012-10-22 260 static int isp_xclk_set_rate(struct clk_hw *hw, unsigned long rate,
9b28ee3c Laurent Pinchart 2012-10-22 261 unsigned long parent_rate)
9b28ee3c Laurent Pinchart 2012-10-22 262 {
9b28ee3c Laurent Pinchart 2012-10-22 263 struct isp_xclk *xclk = to_isp_xclk(hw);
9b28ee3c Laurent Pinchart 2012-10-22 264 unsigned long flags;
9b28ee3c Laurent Pinchart 2012-10-22 265 u32 divider;
9b28ee3c Laurent Pinchart 2012-10-22 266
9b28ee3c Laurent Pinchart 2012-10-22 267 divider = isp_xclk_calc_divider(&rate, parent_rate);
9b28ee3c Laurent Pinchart 2012-10-22 268
9b28ee3c Laurent Pinchart 2012-10-22 269 spin_lock_irqsave(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 270
9b28ee3c Laurent Pinchart 2012-10-22 271 xclk->divider = divider;
9b28ee3c Laurent Pinchart 2012-10-22 272 if (xclk->enabled)
9b28ee3c Laurent Pinchart 2012-10-22 273 isp_xclk_update(xclk, divider);
9b28ee3c Laurent Pinchart 2012-10-22 274
9b28ee3c Laurent Pinchart 2012-10-22 275 spin_unlock_irqrestore(&xclk->lock, flags);
9b28ee3c Laurent Pinchart 2012-10-22 276
9b28ee3c Laurent Pinchart 2012-10-22 277 dev_dbg(xclk->isp->dev, "%s: cam_xclk%c set to %lu Hz (div %u)\n",
9b28ee3c Laurent Pinchart 2012-10-22 278 __func__, xclk->id == ISP_XCLK_A ? 'a' : 'b', rate, divider);
9b28ee3c Laurent Pinchart 2012-10-22 279 return 0;
9b28ee3c Laurent Pinchart 2012-10-22 280 }
9b28ee3c Laurent Pinchart 2012-10-22 281
9b28ee3c Laurent Pinchart 2012-10-22 282 static const struct clk_ops isp_xclk_ops = {
9b28ee3c Laurent Pinchart 2012-10-22 @283 .prepare = isp_xclk_prepare,
9b28ee3c Laurent Pinchart 2012-10-22 @284 .unprepare = isp_xclk_unprepare,
9b28ee3c Laurent Pinchart 2012-10-22 @285 .enable = isp_xclk_enable,
9b28ee3c Laurent Pinchart 2012-10-22 @286 .disable = isp_xclk_disable,
9b28ee3c Laurent Pinchart 2012-10-22 @287 .recalc_rate = isp_xclk_recalc_rate,
9b28ee3c Laurent Pinchart 2012-10-22 @288 .round_rate = isp_xclk_round_rate,
9b28ee3c Laurent Pinchart 2012-10-22 @289 .set_rate = isp_xclk_set_rate,
9b28ee3c Laurent Pinchart 2012-10-22 290 };
9b28ee3c Laurent Pinchart 2012-10-22 291
9b28ee3c Laurent Pinchart 2012-10-22 292 static const char *isp_xclk_parent_name = "cam_mclk";
9b28ee3c Laurent Pinchart 2012-10-22 293
9b28ee3c Laurent Pinchart 2012-10-22 294 static const struct clk_init_data isp_xclk_init_data = {
9b28ee3c Laurent Pinchart 2012-10-22 295 .name = "cam_xclk",
9b28ee3c Laurent Pinchart 2012-10-22 296 .ops = &isp_xclk_ops,
9b28ee3c Laurent Pinchart 2012-10-22 297 .parent_names = &isp_xclk_parent_name,
9b28ee3c Laurent Pinchart 2012-10-22 @298 .num_parents = 1,
9b28ee3c Laurent Pinchart 2012-10-22 299 };
9b28ee3c Laurent Pinchart 2012-10-22 300
64904b57 Laurent Pinchart 2015-03-25 301 static struct clk *isp_xclk_src_get(struct of_phandle_args *clkspec, void *data)
64904b57 Laurent Pinchart 2015-03-25 302 {
64904b57 Laurent Pinchart 2015-03-25 303 unsigned int idx = clkspec->args[0];
64904b57 Laurent Pinchart 2015-03-25 304 struct isp_device *isp = data;
64904b57 Laurent Pinchart 2015-03-25 305
64904b57 Laurent Pinchart 2015-03-25 306 if (idx >= ARRAY_SIZE(isp->xclks))
64904b57 Laurent Pinchart 2015-03-25 307 return ERR_PTR(-ENOENT);
64904b57 Laurent Pinchart 2015-03-25 308
64904b57 Laurent Pinchart 2015-03-25 309 return isp->xclks[idx].clk;
64904b57 Laurent Pinchart 2015-03-25 310 }
64904b57 Laurent Pinchart 2015-03-25 311
9b28ee3c Laurent Pinchart 2012-10-22 312 static int isp_xclk_init(struct isp_device *isp)
9b28ee3c Laurent Pinchart 2012-10-22 313 {
64904b57 Laurent Pinchart 2015-03-25 314 struct device_node *np = isp->dev->of_node;
9b28ee3c Laurent Pinchart 2012-10-22 @315 struct clk_init_data init;
9b28ee3c Laurent Pinchart 2012-10-22 316 unsigned int i;
9b28ee3c Laurent Pinchart 2012-10-22 317
f8e2ff26 Sylwester Nawrocki 2013-12-04 318 for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i)
f8e2ff26 Sylwester Nawrocki 2013-12-04 319 isp->xclks[i].clk = ERR_PTR(-EINVAL);
f8e2ff26 Sylwester Nawrocki 2013-12-04 320
9b28ee3c Laurent Pinchart 2012-10-22 321 for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i) {
9b28ee3c Laurent Pinchart 2012-10-22 322 struct isp_xclk *xclk = &isp->xclks[i];
9b28ee3c Laurent Pinchart 2012-10-22 323
9b28ee3c Laurent Pinchart 2012-10-22 324 xclk->isp = isp;
9b28ee3c Laurent Pinchart 2012-10-22 325 xclk->id = i == 0 ? ISP_XCLK_A : ISP_XCLK_B;
9b28ee3c Laurent Pinchart 2012-10-22 326 xclk->divider = 1;
9b28ee3c Laurent Pinchart 2012-10-22 327 spin_lock_init(&xclk->lock);
9b28ee3c Laurent Pinchart 2012-10-22 328
9b28ee3c Laurent Pinchart 2012-10-22 329 init.name = i == 0 ? "cam_xclka" : "cam_xclkb";
9b28ee3c Laurent Pinchart 2012-10-22 330 init.ops = &isp_xclk_ops;
9b28ee3c Laurent Pinchart 2012-10-22 331 init.parent_names = &isp_xclk_parent_name;
9b28ee3c Laurent Pinchart 2012-10-22 332 init.num_parents = 1;
9b28ee3c Laurent Pinchart 2012-10-22 333
9b28ee3c Laurent Pinchart 2012-10-22 334 xclk->hw.init = &init;
f8e2ff26 Sylwester Nawrocki 2013-12-04 335 /*
f8e2ff26 Sylwester Nawrocki 2013-12-04 336 * The first argument is NULL in order to avoid circular
f8e2ff26 Sylwester Nawrocki 2013-12-04 337 * reference, as this driver takes reference on the
f8e2ff26 Sylwester Nawrocki 2013-12-04 338 * sensor subdevice modules and the sensors would take
f8e2ff26 Sylwester Nawrocki 2013-12-04 339 * reference on this module through clk_get().
f8e2ff26 Sylwester Nawrocki 2013-12-04 340 */
f8e2ff26 Sylwester Nawrocki 2013-12-04 @341 xclk->clk = clk_register(NULL, &xclk->hw);
f8e2ff26 Sylwester Nawrocki 2013-12-04 342 if (IS_ERR(xclk->clk))
f8e2ff26 Sylwester Nawrocki 2013-12-04 343 return PTR_ERR(xclk->clk);
9b28ee3c Laurent Pinchart 2012-10-22 344 }
9b28ee3c Laurent Pinchart 2012-10-22 345
64904b57 Laurent Pinchart 2015-03-25 346 if (np)
64904b57 Laurent Pinchart 2015-03-25 @347 of_clk_add_provider(np, isp_xclk_src_get, isp);
64904b57 Laurent Pinchart 2015-03-25 348
9b28ee3c Laurent Pinchart 2012-10-22 349 return 0;
9b28ee3c Laurent Pinchart 2012-10-22 350 }
9b28ee3c Laurent Pinchart 2012-10-22 351
9b28ee3c Laurent Pinchart 2012-10-22 352 static void isp_xclk_cleanup(struct isp_device *isp)
9b28ee3c Laurent Pinchart 2012-10-22 353 {
64904b57 Laurent Pinchart 2015-03-25 354 struct device_node *np = isp->dev->of_node;
9b28ee3c Laurent Pinchart 2012-10-22 355 unsigned int i;
9b28ee3c Laurent Pinchart 2012-10-22 356
64904b57 Laurent Pinchart 2015-03-25 357 if (np)
64904b57 Laurent Pinchart 2015-03-25 @358 of_clk_del_provider(np);
64904b57 Laurent Pinchart 2015-03-25 359
9b28ee3c Laurent Pinchart 2012-10-22 360 for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i) {
9b28ee3c Laurent Pinchart 2012-10-22 361 struct isp_xclk *xclk = &isp->xclks[i];
9b28ee3c Laurent Pinchart 2012-10-22 362
f8e2ff26 Sylwester Nawrocki 2013-12-04 363 if (!IS_ERR(xclk->clk))
f8e2ff26 Sylwester Nawrocki 2013-12-04 @364 clk_unregister(xclk->clk);
9b28ee3c Laurent Pinchart 2012-10-22 365 }
9b28ee3c Laurent Pinchart 2012-10-22 366 }
9b28ee3c Laurent Pinchart 2012-10-22 367
:::::: The code at line 283 was first introduced by commit
:::::: 9b28ee3c9122cea62f2db02f5bb1e1606bb343a6 [media] omap3isp: Use the common clock framework
:::::: TO: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Mauro, On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote: > Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu: > > On Thursday, 5 April 2018 20:54:02 EEST Mauro Carvalho Chehab wrote: > > > There aren't much things required for it to build with COMPILE_TEST. > > > It just needs to provide stub for an arm-dependent include. > > > > > > Let's replicate the same solution used by ipmmu-vmsa, in order > > > to allow building omap3 with COMPILE_TEST. > > > > > > The actual logic here came from this driver: > > > drivers/iommu/ipmmu-vmsa.c > > > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > > > --- > > > > > > drivers/media/platform/Kconfig | 8 ++++---- > > > drivers/media/platform/omap3isp/isp.c | 7 +++++++ > > > 2 files changed, 11 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/media/platform/Kconfig > > > b/drivers/media/platform/Kconfig index c7a1cf8a1b01..03c9dfeb7781 > > > 100644 > > > --- a/drivers/media/platform/Kconfig > > > +++ b/drivers/media/platform/Kconfig > > > @@ -62,12 +62,12 @@ config VIDEO_MUX > > > > > > config VIDEO_OMAP3 > > > tristate "OMAP 3 Camera support" > > > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > > > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > > > depends on HAS_DMA && OF > > > - depends on OMAP_IOMMU > > > - select ARM_DMA_USE_IOMMU > > > + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST) > > > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU > > > select VIDEOBUF2_DMA_CONTIG > > > - select MFD_SYSCON > > > + select MFD_SYSCON if ARCH_OMAP3 > > > select V4L2_FWNODE > > > ---help--- > > > Driver for an OMAP 3 camera controller. > > > diff --git a/drivers/media/platform/omap3isp/isp.c > > > b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f > > > 100644 > > > --- a/drivers/media/platform/omap3isp/isp.c > > > +++ b/drivers/media/platform/omap3isp/isp.c > > > @@ -61,7 +61,14 @@ > > > #include <linux/sched.h> > > > #include <linux/vmalloc.h> > > > > > > +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA) > > > #include <asm/dma-iommu.h> > > > +#else > > > +#define arm_iommu_create_mapping(...) NULL > > > +#define arm_iommu_attach_device(...) -ENODEV > > > +#define arm_iommu_release_mapping(...) do {} while (0) > > > +#define arm_iommu_detach_device(...) do {} while (0) > > > +#endif > > > > I don't think it's the job of a driver to define those stubs, sorry. > > Otherwise where do you stop ? If you have half of the code that is > > architecture- dependent, would you stub it ? And what if the stubs you > > define here generate warnings in static analyzers ? > > I agree that we should avoid doing that as a general case, but see > below. > > > If you want to make drivers compile for all architectures, the APIs they > > use must be defined in linux/, not in asm/. They can be stubbed there > > when not implemented in a particular architecture, but not in the driver. > > In this specific case, the same approach taken here is already needed > by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside > drivers/iommu: > > drivers/iommu/ipmmu-vmsa.c The reason there is different, the driver is shared by ARM32 and ARM64 platforms. Furthermore, there's an effort (or at least there was) to move away from those APIs in the driver, but I think it has stalled. > Also, this API is used only by 3 drivers [1]: > > drivers/iommu/ipmmu-vmsa.c > drivers/iommu/mtk_iommu_v1.c > drivers/media/platform/omap3isp/isp.c > > [1] as blamed by > git grep -l arm_iommu_create_mapping The exynos driver also uses it. > That hardly seems to be an arch-specific iommu solution, but, instead, some > hack used by only three drivers or some legacy iommu binding. It's more complex than that. There are multiple IOMMU-related APIs on ARM, so more recent than others, with different feature sets. While I agree that drivers should move away from arm_iommu_create_mapping(), doing so requires coordination between the IOMMU driver and the bus master driver (for instance the omap3isp driver). It's not a trivial matter, but I'd love if someone submitted patches :-) > The omap3isp is, btw, the only driver outside drivers/iommu that needs it. > > So, it sounds that other driver uses some other approach, but hardly > it would be worth to change this driver to use something else. > > So, better to stick with the same solution the Renesas driver used. I'm not responsible for that solution and I didn't think it was a good one at the time it was introduced, but in any case it is not meant at all to support COMPILE_TEST. I still don't think the omap3isp driver should declare stubs for these functions for the purpose of supporting compile-testing on x86.
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index c7a1cf8a1b01..03c9dfeb7781 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -62,12 +62,12 @@ config VIDEO_MUX config VIDEO_OMAP3 tristate "OMAP 3 Camera support" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API depends on HAS_DMA && OF - depends on OMAP_IOMMU - select ARM_DMA_USE_IOMMU + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST) + select ARM_DMA_USE_IOMMU if OMAP_IOMMU select VIDEOBUF2_DMA_CONTIG - select MFD_SYSCON + select MFD_SYSCON if ARCH_OMAP3 select V4L2_FWNODE ---help--- Driver for an OMAP 3 camera controller. diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -61,7 +61,14 @@ #include <linux/sched.h> #include <linux/vmalloc.h> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA) #include <asm/dma-iommu.h> +#else +#define arm_iommu_create_mapping(...) NULL +#define arm_iommu_attach_device(...) -ENODEV +#define arm_iommu_release_mapping(...) do {} while (0) +#define arm_iommu_detach_device(...) do {} while (0) +#endif #include <media/v4l2-common.h> #include <media/v4l2-fwnode.h>
There aren't much things required for it to build with COMPILE_TEST. It just needs to provide stub for an arm-dependent include. Let's replicate the same solution used by ipmmu-vmsa, in order to allow building omap3 with COMPILE_TEST. The actual logic here came from this driver: drivers/iommu/ipmmu-vmsa.c Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- drivers/media/platform/Kconfig | 8 ++++---- drivers/media/platform/omap3isp/isp.c | 7 +++++++ 2 files changed, 11 insertions(+), 4 deletions(-)