Skip to content

Commit

Permalink
media: atmel: atmel-isc: move to staging
Browse files Browse the repository at this point in the history
The Atmel ISC driver is not compliant with media controller specification.
In order to evolve this driver, it has to move to media controller, to
support enhanced features and future products which embed it.
The move to media controller involves several changes which are
not backwards compatible with the current usability of the driver.

The best example is the way the format is propagated from the top video
driver /dev/videoX down to the sensor.

In a simple configuration sensor ==> isc , the isc just calls subdev s_fmt
and controls the sensor directly. This is achieved by having a lot of code
inside the driver that will query the subdev at probe time and make a list
of formats which are usable.
Basically the user has nothing to configure, as the isc will handle
everything at the top level. This is an easy way to capture, but also comes
with the drawback of lack of flexibility.
In a more complicated pipeline
sensor ==> controller 1 ==> controller 2 ==> isc
this will not be achievable, as controller 1 and controller 2 might be
media-controller configurable, and will not propagate the formats down to
the sensor.

After discussions with the media maintainers, the decision is to move
Atmel ISC to staging as-is, to keep the Kconfig symbols and the users
to the driver in staging. Thus, all the existing users of the non
media-controller paradigm will continue to be happy and use the old config
way.

The new driver was added in the media subsystem with a different
symbol, with the conversion to media controller done, and new users
of the driver will be able to use all the new features.

This patch is merely a file move to staging, not affecting any of the
users.

The exported symbols had to be renamed to atmel_* to avoid duplication with
the new Microchip ISC driver.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
  • Loading branch information
ehristev authored and mchehab committed Nov 25, 2022
1 parent 8a8f9ce commit 55927c9
Show file tree
Hide file tree
Showing 14 changed files with 133 additions and 84 deletions.
4 changes: 2 additions & 2 deletions MAINTAINERS
Original file line number Diff line number Diff line change
Expand Up @@ -13503,8 +13503,8 @@ L: linux-media@vger.kernel.org
S: Supported
F: Documentation/devicetree/bindings/media/atmel,isc.yaml
F: Documentation/devicetree/bindings/media/microchip,xisc.yaml
F: drivers/media/platform/atmel/atmel-isc*
F: drivers/media/platform/atmel/atmel-sama*-isc*
F: drivers/staging/media/deprecated/atmel/atmel-isc*
F: drivers/staging/media/deprecated/atmel/atmel-sama*-isc*
F: drivers/media/platform/microchip/microchip-isc*
F: drivers/media/platform/microchip/microchip-sama*-isc*
F: include/linux/atmel-isc-media.h
Expand Down
36 changes: 0 additions & 36 deletions drivers/media/platform/atmel/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,6 @@

comment "Atmel media platform drivers"

config VIDEO_ATMEL_ISC
tristate "ATMEL Image Sensor Controller (ISC) support"
depends on V4L_PLATFORM_DRIVERS
depends on VIDEO_DEV && COMMON_CLK
depends on ARCH_AT91 || COMPILE_TEST
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_DMA_CONTIG
select REGMAP_MMIO
select V4L2_FWNODE
select VIDEO_ATMEL_ISC_BASE
help
This module makes the ATMEL Image Sensor Controller available
as a v4l2 device.

config VIDEO_ATMEL_XISC
tristate "ATMEL eXtended Image Sensor Controller (XISC) support"
depends on V4L_PLATFORM_DRIVERS
depends on VIDEO_DEV && COMMON_CLK
depends on ARCH_AT91 || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select REGMAP_MMIO
select V4L2_FWNODE
select VIDEO_ATMEL_ISC_BASE
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
help
This module makes the ATMEL eXtended Image Sensor Controller
available as a v4l2 device.

config VIDEO_ATMEL_ISC_BASE
tristate
default n
help
ATMEL ISC and XISC common code base.

config VIDEO_ATMEL_ISI
tristate "ATMEL Image Sensor Interface (ISI) support"
depends on V4L_PLATFORM_DRIVERS
Expand Down
6 changes: 0 additions & 6 deletions drivers/media/platform/atmel/Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
atmel-isc-objs = atmel-sama5d2-isc.o
atmel-xisc-objs = atmel-sama7g5-isc.o
atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o

obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-common.o
obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel-xisc.o
1 change: 1 addition & 0 deletions drivers/staging/media/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ menuconfig STAGING_MEDIA_DEPRECATED
If in doubt, say N here.

if STAGING_MEDIA_DEPRECATED
source "drivers/staging/media/deprecated/atmel/Kconfig"
source "drivers/staging/media/deprecated/cpia2/Kconfig"
source "drivers/staging/media/deprecated/fsl-viu/Kconfig"
source "drivers/staging/media/deprecated/meye/Kconfig"
Expand Down
1 change: 1 addition & 0 deletions drivers/staging/media/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += deprecated/atmel/
obj-$(CONFIG_INTEL_ATOMISP) += atomisp/
obj-$(CONFIG_VIDEO_CPIA2) += deprecated/cpia2/
obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx/
Expand Down
47 changes: 47 additions & 0 deletions drivers/staging/media/deprecated/atmel/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# SPDX-License-Identifier: GPL-2.0-only

comment "Atmel media platform drivers"

config VIDEO_ATMEL_ISC
tristate "ATMEL Image Sensor Controller (ISC) support (DEPRECATED)"
depends on V4L_PLATFORM_DRIVERS
depends on VIDEO_DEV && COMMON_CLK
depends on ARCH_AT91 || COMPILE_TEST
depends on !VIDEO_MICROCHIP_ISC_BASE || COMPILE_TEST
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_DMA_CONTIG
select REGMAP_MMIO
select V4L2_FWNODE
select VIDEO_ATMEL_ISC_BASE
help
This module makes the ATMEL Image Sensor Controller available
as a v4l2 device.

This driver is deprecated and is scheduled for removal by
the beginning of 2026. See the TODO file for more information.

config VIDEO_ATMEL_XISC
tristate "ATMEL eXtended Image Sensor Controller (XISC) support (DEPRECATED)"
depends on V4L_PLATFORM_DRIVERS
depends on VIDEO_DEV && COMMON_CLK
depends on ARCH_AT91 || COMPILE_TEST
depends on !VIDEO_MICROCHIP_ISC_BASE || COMPILE_TEST
select VIDEOBUF2_DMA_CONTIG
select REGMAP_MMIO
select V4L2_FWNODE
select VIDEO_ATMEL_ISC_BASE
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
help
This module makes the ATMEL eXtended Image Sensor Controller
available as a v4l2 device.

This driver is deprecated and is scheduled for removal by
the beginning of 2026. See the TODO file for more information.

config VIDEO_ATMEL_ISC_BASE
tristate
default n
help
ATMEL ISC and XISC common code base.
8 changes: 8 additions & 0 deletions drivers/staging/media/deprecated/atmel/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# SPDX-License-Identifier: GPL-2.0-only
atmel-isc-objs = atmel-sama5d2-isc.o
atmel-xisc-objs = atmel-sama7g5-isc.o
atmel-isc-common-objs = atmel-isc-base.o atmel-isc-clk.o

obj-$(CONFIG_VIDEO_ATMEL_ISC_BASE) += atmel-isc-common.o
obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
obj-$(CONFIG_VIDEO_ATMEL_XISC) += atmel-xisc.o
34 changes: 34 additions & 0 deletions drivers/staging/media/deprecated/atmel/TODO
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
The Atmel ISC driver is not compliant with media controller specification.
In order to evolve this driver, it has to move to media controller, to
support enhanced features and future products which embed it.
The move to media controller involves several changes which are
not backwards compatible with the current usability of the driver.

The best example is the way the format is propagated from the top video
driver /dev/videoX down to the sensor.

In a simple configuration sensor ==> isc , the isc just calls subdev s_fmt
and controls the sensor directly. This is achieved by having a lot of code
inside the driver that will query the subdev at probe time and make a list
of formats which are usable.
Basically the user has nothing to configure, as the isc will handle
everything at the top level. This is an easy way to capture, but also comes
with the drawback of lack of flexibility.
In a more complicated pipeline
sensor ==> controller 1 ==> controller 2 ==> isc
this will not be achievable, as controller 1 and controller 2 might be
media-controller configurable, and will not propagate the formats down to
the sensor.

After discussions with the media maintainers, the decision is to move
Atmel ISC to staging as-is, to keep the Kconfig symbols and the users
to the driver in staging. Thus, all the existing users of the non
media-controller paradigm will continue to be happy and use the old config
way.

The new driver was added in the media subsystem with a different
symbol, with the conversion to media controller done, and new users
of the driver will be able to use all the new features.

The replacement driver is named VIDEO_MICROCHIP_ISC or
VIDEO_MICROCHIP_XISC depending on the product flavor.
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ static const struct v4l2_file_operations isc_fops = {
.poll = vb2_fop_poll,
};

irqreturn_t isc_interrupt(int irq, void *dev_id)
irqreturn_t atmel_isc_interrupt(int irq, void *dev_id)
{
struct isc_device *isc = (struct isc_device *)dev_id;
struct regmap *regmap = isc->regmap;
Expand Down Expand Up @@ -1267,7 +1267,7 @@ irqreturn_t isc_interrupt(int irq, void *dev_id)

return ret;
}
EXPORT_SYMBOL_GPL(isc_interrupt);
EXPORT_SYMBOL_GPL(atmel_isc_interrupt);

static void isc_hist_count(struct isc_device *isc, u32 *min, u32 *max)
{
Expand Down Expand Up @@ -1934,14 +1934,14 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
return ret;
}

const struct v4l2_async_notifier_operations isc_async_ops = {
const struct v4l2_async_notifier_operations atmel_isc_async_ops = {
.bound = isc_async_bound,
.unbind = isc_async_unbind,
.complete = isc_async_complete,
};
EXPORT_SYMBOL_GPL(isc_async_ops);
EXPORT_SYMBOL_GPL(atmel_isc_async_ops);

void isc_subdev_cleanup(struct isc_device *isc)
void atmel_isc_subdev_cleanup(struct isc_device *isc)
{
struct isc_subdev_entity *subdev_entity;

Expand All @@ -1952,9 +1952,9 @@ void isc_subdev_cleanup(struct isc_device *isc)

INIT_LIST_HEAD(&isc->subdev_entities);
}
EXPORT_SYMBOL_GPL(isc_subdev_cleanup);
EXPORT_SYMBOL_GPL(atmel_isc_subdev_cleanup);

int isc_pipeline_init(struct isc_device *isc)
int atmel_isc_pipeline_init(struct isc_device *isc)
{
struct device *dev = isc->dev;
struct regmap *regmap = isc->regmap;
Expand Down Expand Up @@ -1993,17 +1993,17 @@ int isc_pipeline_init(struct isc_device *isc)

return 0;
}
EXPORT_SYMBOL_GPL(isc_pipeline_init);
EXPORT_SYMBOL_GPL(atmel_isc_pipeline_init);

/* regmap configuration */
#define ATMEL_ISC_REG_MAX 0xd5c
const struct regmap_config isc_regmap_config = {
const struct regmap_config atmel_isc_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
.val_bits = 32,
.max_register = ATMEL_ISC_REG_MAX,
};
EXPORT_SYMBOL_GPL(isc_regmap_config);
EXPORT_SYMBOL_GPL(atmel_isc_regmap_config);

MODULE_AUTHOR("Songjun Wu");
MODULE_AUTHOR("Eugen Hristev");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ static int isc_clk_register(struct isc_device *isc, unsigned int id)
return 0;
}

int isc_clk_init(struct isc_device *isc)
int atmel_isc_clk_init(struct isc_device *isc)
{
unsigned int i;
int ret;
Expand All @@ -293,9 +293,9 @@ int isc_clk_init(struct isc_device *isc)

return 0;
}
EXPORT_SYMBOL_GPL(isc_clk_init);
EXPORT_SYMBOL_GPL(atmel_isc_clk_init);

void isc_clk_cleanup(struct isc_device *isc)
void atmel_isc_clk_cleanup(struct isc_device *isc)
{
unsigned int i;

Expand All @@ -308,4 +308,4 @@ void isc_clk_cleanup(struct isc_device *isc)
clk_unregister(isc_clk->clk);
}
}
EXPORT_SYMBOL_GPL(isc_clk_cleanup);
EXPORT_SYMBOL_GPL(atmel_isc_clk_cleanup);
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,13 @@ struct isc_device {
u32 formats_list_size;
};

extern const struct regmap_config isc_regmap_config;
extern const struct v4l2_async_notifier_operations isc_async_ops;

irqreturn_t isc_interrupt(int irq, void *dev_id);
int isc_pipeline_init(struct isc_device *isc);
int isc_clk_init(struct isc_device *isc);
void isc_subdev_cleanup(struct isc_device *isc);
void isc_clk_cleanup(struct isc_device *isc);
extern const struct regmap_config atmel_isc_regmap_config;
extern const struct v4l2_async_notifier_operations atmel_isc_async_ops;

irqreturn_t atmel_isc_interrupt(int irq, void *dev_id);
int atmel_isc_pipeline_init(struct isc_device *isc);
int atmel_isc_clk_init(struct isc_device *isc);
void atmel_isc_subdev_cleanup(struct isc_device *isc);
void atmel_isc_clk_cleanup(struct isc_device *isc);

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
if (IS_ERR(io_base))
return PTR_ERR(io_base);

isc->regmap = devm_regmap_init_mmio(dev, io_base, &isc_regmap_config);
isc->regmap = devm_regmap_init_mmio(dev, io_base, &atmel_isc_regmap_config);
if (IS_ERR(isc->regmap)) {
ret = PTR_ERR(isc->regmap);
dev_err(dev, "failed to init register map: %d\n", ret);
Expand All @@ -419,7 +419,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
if (irq < 0)
return irq;

ret = devm_request_irq(dev, irq, isc_interrupt, 0,
ret = devm_request_irq(dev, irq, atmel_isc_interrupt, 0,
"atmel-sama5d2-isc", isc);
if (ret < 0) {
dev_err(dev, "can't register ISR for IRQ %u (ret=%i)\n",
Expand Down Expand Up @@ -464,7 +464,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
/* sama5d2-isc : ISPCK is required and mandatory */
isc->ispck_required = true;

ret = isc_pipeline_init(isc);
ret = atmel_isc_pipeline_init(isc);
if (ret)
return ret;

Expand All @@ -481,7 +481,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
return ret;
}

ret = isc_clk_init(isc);
ret = atmel_isc_clk_init(isc);
if (ret) {
dev_err(dev, "failed to init isc clock: %d\n", ret);
goto unprepare_hclk;
Expand Down Expand Up @@ -523,7 +523,7 @@ static int atmel_isc_probe(struct platform_device *pdev)
goto cleanup_subdev;
}

subdev_entity->notifier.ops = &isc_async_ops;
subdev_entity->notifier.ops = &atmel_isc_async_ops;

ret = v4l2_async_nf_register(&isc->v4l2_dev,
&subdev_entity->notifier);
Expand Down Expand Up @@ -567,15 +567,15 @@ static int atmel_isc_probe(struct platform_device *pdev)
pm_runtime_disable(dev);

cleanup_subdev:
isc_subdev_cleanup(isc);
atmel_isc_subdev_cleanup(isc);

unregister_v4l2_device:
v4l2_device_unregister(&isc->v4l2_dev);

unprepare_hclk:
clk_disable_unprepare(isc->hclock);

isc_clk_cleanup(isc);
atmel_isc_clk_cleanup(isc);

return ret;
}
Expand All @@ -586,14 +586,14 @@ static int atmel_isc_remove(struct platform_device *pdev)

pm_runtime_disable(&pdev->dev);

isc_subdev_cleanup(isc);
atmel_isc_subdev_cleanup(isc);

v4l2_device_unregister(&isc->v4l2_dev);

clk_disable_unprepare(isc->ispck);
clk_disable_unprepare(isc->hclock);

isc_clk_cleanup(isc);
atmel_isc_clk_cleanup(isc);

return 0;
}
Expand Down
Loading

0 comments on commit 55927c9

Please sign in to comment.