Message ID | 20230824080109.89613-4-jack.zhu@starfivetech.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add StarFive Camera Subsystem driver | expand |
Le 24/08/2023 à 10:01, Jack Zhu a écrit : > Add core driver for StarFive Camera Subsystem. The code parses > the device platform resources and registers related devices. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com> > --- ... > diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig > new file mode 100644 > index 000000000000..8d20e2bd2559 > --- /dev/null > +++ b/drivers/staging/media/starfive/camss/Kconfig > @@ -0,0 +1,17 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config VIDEO_STARFIVE_CAMSS > + tristate "Starfive Camera Subsystem driver" > + depends on V4L_PLATFORM_DRIVERS > + depends on VIDEO_DEV && OF > + depends on HAS_DMA > + depends on PM > + select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_FWNODE > + help > + Enable this to support for the Starfive Camera subsystem > + found on Starfive JH7110 SoC. > + > + To compile this driver as a module, choose M here: the > + module will be called stf-camss. stf_camss? (s/-/_) > diff --git a/drivers/staging/media/starfive/camss/Makefile b/drivers/staging/media/starfive/camss/Makefile > new file mode 100644 > index 000000000000..f53c5cbe958f > --- /dev/null > +++ b/drivers/staging/media/starfive/camss/Makefile > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Makefile for StarFive Camera Subsystem driver > +# > + > +starfive-camss-objs += \ > + stf_camss.o > + > +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o I'm not an expert in Makefile files, but this stf_camss.o and starfive-camss.o look strange to me. > diff --git a/drivers/staging/media/starfive/camss/stf_camss.c b/drivers/staging/media/starfive/camss/stf_camss.c > new file mode 100644 > index 000000000000..75ebc3a35218 > --- /dev/null > +++ b/drivers/staging/media/starfive/camss/stf_camss.c ... > +static int stfcamss_of_parse_ports(struct stfcamss *stfcamss) > +{ > + struct device_node *node = NULL; > + int ret, num_subdevs = 0; > + > + for_each_endpoint_of_node(stfcamss->dev->of_node, node) { > + struct stfcamss_async_subdev *csd; > + > + if (!of_device_is_available(node)) > + continue; > + > + csd = v4l2_async_nf_add_fwnode_remote(&stfcamss->notifier, > + of_fwnode_handle(node), > + struct stfcamss_async_subdev); > + if (IS_ERR(csd)) { > + ret = PTR_ERR(csd); > + dev_err(stfcamss->dev, "failed to add async notifier\n"); > + v4l2_async_nf_cleanup(&stfcamss->notifier); having it here, looks strange to me. It is already called in the error handling path of the probe. Should there be a "of_node_put(node);" if we return here? > + return ret; > + } > + > + ret = stfcamss_of_parse_endpoint_node(stfcamss, node, csd); > + if (ret) > + return ret; > + > + num_subdevs++; > + } > + > + return num_subdevs; > +} ... > +static int stfcamss_remove(struct platform_device *pdev) > +{ > + struct stfcamss *stfcamss = platform_get_drvdata(pdev); > + > + v4l2_device_unregister(&stfcamss->v4l2_dev); > + media_device_cleanup(&stfcamss->media_dev); Is a "v4l2_async_nf_cleanup(&stfcamss->notifier);" missing to match the error handling path of the probe? > + pm_runtime_disable(&pdev->dev); > + > + return 0; > +} > + ...
Hi Christophe, Thank you for your comment! On 2023/8/25 2:31, Christophe JAILLET wrote: > Le 24/08/2023 à 10:01, Jack Zhu a écrit : >> Add core driver for StarFive Camera Subsystem. The code parses >> the device platform resources and registers related devices. >> >> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com> >> --- > > ... > >> diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig >> new file mode 100644 >> index 000000000000..8d20e2bd2559 >> --- /dev/null >> +++ b/drivers/staging/media/starfive/camss/Kconfig >> @@ -0,0 +1,17 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +config VIDEO_STARFIVE_CAMSS >> + tristate "Starfive Camera Subsystem driver" >> + depends on V4L_PLATFORM_DRIVERS >> + depends on VIDEO_DEV && OF >> + depends on HAS_DMA >> + depends on PM >> + select MEDIA_CONTROLLER >> + select VIDEO_V4L2_SUBDEV_API >> + select VIDEOBUF2_DMA_CONTIG >> + select V4L2_FWNODE >> + help >> + Enable this to support for the Starfive Camera subsystem >> + found on Starfive JH7110 SoC. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called stf-camss. > > stf_camss? (s/-/_) > Refer to the writing method of other media drivers, most of them use hyphen. It may be better to use ‘starfive-camss'? >> diff --git a/drivers/staging/media/starfive/camss/Makefile b/drivers/staging/media/starfive/camss/Makefile >> new file mode 100644 >> index 000000000000..f53c5cbe958f >> --- /dev/null >> +++ b/drivers/staging/media/starfive/camss/Makefile >> @@ -0,0 +1,9 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +# >> +# Makefile for StarFive Camera Subsystem driver >> +# >> + >> +starfive-camss-objs += \ >> + stf_camss.o >> + >> +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o > > I'm not an expert in Makefile files, but this stf_camss.o and starfive-camss.o look strange to me. > Is it better to replace 'stf_camss.o' with 'stf-camss.o', which is consistent with the driving style of other media drivers? >> diff --git a/drivers/staging/media/starfive/camss/stf_camss.c b/drivers/staging/media/starfive/camss/stf_camss.c >> new file mode 100644 >> index 000000000000..75ebc3a35218 >> --- /dev/null >> +++ b/drivers/staging/media/starfive/camss/stf_camss.c > > ... > >> +static int stfcamss_of_parse_ports(struct stfcamss *stfcamss) >> +{ >> + struct device_node *node = NULL; >> + int ret, num_subdevs = 0; >> + >> + for_each_endpoint_of_node(stfcamss->dev->of_node, node) { >> + struct stfcamss_async_subdev *csd; >> + >> + if (!of_device_is_available(node)) >> + continue; >> + >> + csd = v4l2_async_nf_add_fwnode_remote(&stfcamss->notifier, >> + of_fwnode_handle(node), >> + struct stfcamss_async_subdev); >> + if (IS_ERR(csd)) { >> + ret = PTR_ERR(csd); >> + dev_err(stfcamss->dev, "failed to add async notifier\n"); >> + v4l2_async_nf_cleanup(&stfcamss->notifier); > > having it here, looks strange to me. > It is already called in the error handling path of the probe. > > Should there be a "of_node_put(node);" if we return here? > We do not call a 'get' interface, is it necessary to use the 'put' interface? >> + return ret; >> + } >> + >> + ret = stfcamss_of_parse_endpoint_node(stfcamss, node, csd); >> + if (ret) >> + return ret; >> + >> + num_subdevs++; >> + } >> + >> + return num_subdevs; >> +} > > ... > >> +static int stfcamss_remove(struct platform_device *pdev) >> +{ >> + struct stfcamss *stfcamss = platform_get_drvdata(pdev); >> + >> + v4l2_device_unregister(&stfcamss->v4l2_dev); >> + media_device_cleanup(&stfcamss->media_dev); > > Is a "v4l2_async_nf_cleanup(&stfcamss->notifier);" missing to match the error handling path of the probe? > >> + pm_runtime_disable(&pdev->dev); >> + >> + return 0; >> +} >> + > > ...
Le 25/08/2023 à 12:44, Jack Zhu a écrit : > Hi Christophe, > > Thank you for your comment! > > On 2023/8/25 2:31, Christophe JAILLET wrote: >> Le 24/08/2023 à 10:01, Jack Zhu a écrit : >>> Add core driver for StarFive Camera Subsystem. The code parses >>> the device platform resources and registers related devices. >>> >>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com> >>> --- >> >> ... >> >>> diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig >>> new file mode 100644 >>> index 000000000000..8d20e2bd2559 >>> --- /dev/null >>> +++ b/drivers/staging/media/starfive/camss/Kconfig >>> @@ -0,0 +1,17 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only >>> +config VIDEO_STARFIVE_CAMSS >>> + tristate "Starfive Camera Subsystem driver" >>> + depends on V4L_PLATFORM_DRIVERS >>> + depends on VIDEO_DEV && OF >>> + depends on HAS_DMA >>> + depends on PM >>> + select MEDIA_CONTROLLER >>> + select VIDEO_V4L2_SUBDEV_API >>> + select VIDEOBUF2_DMA_CONTIG >>> + select V4L2_FWNODE >>> + help >>> + Enable this to support for the Starfive Camera subsystem >>> + found on Starfive JH7110 SoC. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called msstf-cas. >> >> stf_camss? (s/-/_) >> > > Refer to the writing method of other media drivers, most of them use hyphen. Forget about my comment. I have been puzzled by "msstf-cas" here, vs "stf_camss" below. > it may be better to use ‘starfive-camss'? Yes, I think so. > >>> diff --git a/drivers/staging/media/starfive/camss/Makefile b/drivers/staging/media/starfive/camss/Makefile >>> new file mode 100644 >>> index 000000000000..f53c5cbe958f >>> --- /dev/null >>> +++ b/drivers/staging/media/starfive/camss/Makefile >>> @@ -0,0 +1,9 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +# >>> +# Makefile for StarFive Camera Subsystem driver >>> +# >>> + >>> +starfive-camss-objs += \ >>> + stf_camss.o >>> + >>> +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o >> >> I'm not an expert in Makefile files, but this stf_camss.o and starfive-camss.o look strange to me. >> > > Is it better to replace 'stf_camss.o' with 'stf-camss.o', which is consistent > with the driving style of other media drivers? No strong opinion on it. Consistency is always good. > >>> diff --git a/drivers/staging/media/starfive/camss/stf_camss.c b/drivers/staging/media/starfive/camss/stf_camss.c >>> new file mode 100644 >>> index 000000000000..75ebc3a35218 >>> --- /dev/null >>> +++ b/drivers/staging/media/starfive/camss/stf_camss.c >> >> ... >> >>> +static int stfcamss_of_parse_ports(struct stfcamss *stfcamss) >>> +{ >>> + struct device_node *node = NULL; >>> + int ret, num_subdevs = 0; >>> + >>> + for_each_endpoint_of_node(stfcamss->dev->of_node, node) { >>> + struct stfcamss_async_subdev *csd; >>> + >>> + if (!of_device_is_available(node)) >>> + continue; >>> + >>> + csd = v4l2_async_nf_add_fwnode_remote(&stfcamss->notifier, >>> + of_fwnode_handle(node), >>> + struct stfcamss_async_subdev); >>> + if (IS_ERR(csd)) { >>> + ret = PTR_ERR(csd); >>> + dev_err(stfcamss->dev, "failed to add async notifier\n"); >>> + v4l2_async_nf_cleanup(&stfcamss->notifier); >> >> having it here, looks strange to me. >> It is already called in the error handling path of the probe. >> >> Should there be a "of_node_put(node);" if we return here? >> > > We do not call a 'get' interface, is it necessary to use the 'put' interface? for_each_endpoint_of_node() does. See [1] for doc, and [2] for an example. [1]: https://elixir.bootlin.com/linux/v6.5-rc7/source/include/linux/of_graph.h#L30 [2]: https://elixir.bootlin.com/linux/v6.5-rc7/source/drivers/gpu/drm/bridge/tc358767.c#L2196 Also, at least because of the recent b8ec754ae4c5 in -next, your patch does not compile as-is on -next. CJ > >>> + return ret; >>> + } >>> + >>> + ret = stfcamss_of_parse_endpoint_node(stfcamss, node, csd); >>> + if (ret) >>> + return ret; >>> + >>> + num_subdevs++; >>> + } >>> + >>> + return num_subdevs; >>> +} >> >> ... >> >>> +static int stfcamss_remove(struct platform_device *pdev) >>> +{ >>> + struct stfcamss *stfcamss = platform_get_drvdata(pdev); >>> + >>> + v4l2_device_unregister(&stfcamss->v4l2_dev); >>> + media_device_cleanup(&stfcamss->media_dev); >> >> Is a "v4l2_async_nf_cleanup(&stfcamss->notifier);" missing to match the error handling path of the probe? >> >>> + pm_runtime_disable(&pdev->dev); >>> + >>> + return 0; >>> +} >>> + >> >> ... >
On 24/08/2023 10:01, Jack Zhu wrote: > Add core driver for StarFive Camera Subsystem. The code parses > the device platform resources and registers related devices. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com> > --- > MAINTAINERS | 1 + > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile | 1 + > drivers/staging/media/starfive/Kconfig | 5 + > drivers/staging/media/starfive/Makefile | 2 + > drivers/staging/media/starfive/camss/Kconfig | 17 + > drivers/staging/media/starfive/camss/Makefile | 9 + > .../staging/media/starfive/camss/stf_camss.c | 316 ++++++++++++++++++ > .../staging/media/starfive/camss/stf_camss.h | 129 +++++++ > 9 files changed, 482 insertions(+) > create mode 100644 drivers/staging/media/starfive/Kconfig > create mode 100644 drivers/staging/media/starfive/Makefile > create mode 100644 drivers/staging/media/starfive/camss/Kconfig > create mode 100644 drivers/staging/media/starfive/camss/Makefile > create mode 100644 drivers/staging/media/starfive/camss/stf_camss.c > create mode 100644 drivers/staging/media/starfive/camss/stf_camss.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4c63c0a85301..97d3054416ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20258,6 +20258,7 @@ L: linux-media@vger.kernel.org > S: Maintained > F: Documentation/admin-guide/media/starfive_camss.rst > F: Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml > +F: drivers/staging/media/starfive/camss > > STARFIVE CRYPTO DRIVER > M: Jia Jie Ho <jiajie.ho@starfivetech.com> > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index bc6c7b248f86..554c2e475ce3 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -36,6 +36,8 @@ source "drivers/staging/media/omap4iss/Kconfig" > > source "drivers/staging/media/rkvdec/Kconfig" > > +source "drivers/staging/media/starfive/Kconfig" > + > source "drivers/staging/media/sunxi/Kconfig" > > source "drivers/staging/media/tegra-video/Kconfig" > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index 1a4c3a062e3d..dcaeeca0ee6d 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_VIDEO_MAX96712) += max96712/ > obj-$(CONFIG_VIDEO_MESON_VDEC) += meson/vdec/ > obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ > obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC) += rkvdec/ > +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive/ > obj-$(CONFIG_VIDEO_SUNXI) += sunxi/ > obj-$(CONFIG_VIDEO_TEGRA) += tegra-video/ > obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3/ > diff --git a/drivers/staging/media/starfive/Kconfig b/drivers/staging/media/starfive/Kconfig > new file mode 100644 > index 000000000000..34727cf56072 > --- /dev/null > +++ b/drivers/staging/media/starfive/Kconfig > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +comment "StarFive media platform drivers" > + > +source "drivers/staging/media/starfive/camss/Kconfig" > diff --git a/drivers/staging/media/starfive/Makefile b/drivers/staging/media/starfive/Makefile > new file mode 100644 > index 000000000000..4639fa1bca32 > --- /dev/null > +++ b/drivers/staging/media/starfive/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-y += camss/ > diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig > new file mode 100644 > index 000000000000..8d20e2bd2559 > --- /dev/null > +++ b/drivers/staging/media/starfive/camss/Kconfig > @@ -0,0 +1,17 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config VIDEO_STARFIVE_CAMSS > + tristate "Starfive Camera Subsystem driver" > + depends on V4L_PLATFORM_DRIVERS > + depends on VIDEO_DEV && OF > + depends on HAS_DMA > + depends on PM > + select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_FWNODE > + help > + Enable this to support for the Starfive Camera subsystem > + found on Starfive JH7110 SoC. > + > + To compile this driver as a module, choose M here: the > + module will be called stf-camss. Erm, isn't it called starfive-camss.ko? Regards, Hans
On 24/08/2023 10:01, Jack Zhu wrote: > Add core driver for StarFive Camera Subsystem. The code parses > the device platform resources and registers related devices. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com> > --- > MAINTAINERS | 1 + > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile | 1 + > drivers/staging/media/starfive/Kconfig | 5 + > drivers/staging/media/starfive/Makefile | 2 + > drivers/staging/media/starfive/camss/Kconfig | 17 + > drivers/staging/media/starfive/camss/Makefile | 9 + > .../staging/media/starfive/camss/stf_camss.c | 316 ++++++++++++++++++ > .../staging/media/starfive/camss/stf_camss.h | 129 +++++++ > 9 files changed, 482 insertions(+) > create mode 100644 drivers/staging/media/starfive/Kconfig > create mode 100644 drivers/staging/media/starfive/Makefile > create mode 100644 drivers/staging/media/starfive/camss/Kconfig > create mode 100644 drivers/staging/media/starfive/camss/Makefile > create mode 100644 drivers/staging/media/starfive/camss/stf_camss.c > create mode 100644 drivers/staging/media/starfive/camss/stf_camss.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4c63c0a85301..97d3054416ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20258,6 +20258,7 @@ L: linux-media@vger.kernel.org > S: Maintained > F: Documentation/admin-guide/media/starfive_camss.rst > F: Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml > +F: drivers/staging/media/starfive/camss > > STARFIVE CRYPTO DRIVER > M: Jia Jie Ho <jiajie.ho@starfivetech.com> > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index bc6c7b248f86..554c2e475ce3 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -36,6 +36,8 @@ source "drivers/staging/media/omap4iss/Kconfig" > > source "drivers/staging/media/rkvdec/Kconfig" > > +source "drivers/staging/media/starfive/Kconfig" > + > source "drivers/staging/media/sunxi/Kconfig" > > source "drivers/staging/media/tegra-video/Kconfig" > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index 1a4c3a062e3d..dcaeeca0ee6d 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_VIDEO_MAX96712) += max96712/ > obj-$(CONFIG_VIDEO_MESON_VDEC) += meson/vdec/ > obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ > obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC) += rkvdec/ > +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive/ > obj-$(CONFIG_VIDEO_SUNXI) += sunxi/ > obj-$(CONFIG_VIDEO_TEGRA) += tegra-video/ > obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3/ > diff --git a/drivers/staging/media/starfive/Kconfig b/drivers/staging/media/starfive/Kconfig > new file mode 100644 > index 000000000000..34727cf56072 > --- /dev/null > +++ b/drivers/staging/media/starfive/Kconfig > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +comment "StarFive media platform drivers" > + > +source "drivers/staging/media/starfive/camss/Kconfig" > diff --git a/drivers/staging/media/starfive/Makefile b/drivers/staging/media/starfive/Makefile > new file mode 100644 > index 000000000000..4639fa1bca32 > --- /dev/null > +++ b/drivers/staging/media/starfive/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-y += camss/ > diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig > new file mode 100644 > index 000000000000..8d20e2bd2559 > --- /dev/null > +++ b/drivers/staging/media/starfive/camss/Kconfig > @@ -0,0 +1,17 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config VIDEO_STARFIVE_CAMSS > + tristate "Starfive Camera Subsystem driver" > + depends on V4L_PLATFORM_DRIVERS > + depends on VIDEO_DEV && OF > + depends on HAS_DMA > + depends on PM > + select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_FWNODE > + help > + Enable this to support for the Starfive Camera subsystem > + found on Starfive JH7110 SoC. > + > + To compile this driver as a module, choose M here: the > + module will be called stf-camss. This is actually called starfive-camss.ko! Regards, Hans
diff --git a/MAINTAINERS b/MAINTAINERS index 4c63c0a85301..97d3054416ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20258,6 +20258,7 @@ L: linux-media@vger.kernel.org S: Maintained F: Documentation/admin-guide/media/starfive_camss.rst F: Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml +F: drivers/staging/media/starfive/camss STARFIVE CRYPTO DRIVER M: Jia Jie Ho <jiajie.ho@starfivetech.com> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig index bc6c7b248f86..554c2e475ce3 100644 --- a/drivers/staging/media/Kconfig +++ b/drivers/staging/media/Kconfig @@ -36,6 +36,8 @@ source "drivers/staging/media/omap4iss/Kconfig" source "drivers/staging/media/rkvdec/Kconfig" +source "drivers/staging/media/starfive/Kconfig" + source "drivers/staging/media/sunxi/Kconfig" source "drivers/staging/media/tegra-video/Kconfig" diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile index 1a4c3a062e3d..dcaeeca0ee6d 100644 --- a/drivers/staging/media/Makefile +++ b/drivers/staging/media/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_VIDEO_MAX96712) += max96712/ obj-$(CONFIG_VIDEO_MESON_VDEC) += meson/vdec/ obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC) += rkvdec/ +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive/ obj-$(CONFIG_VIDEO_SUNXI) += sunxi/ obj-$(CONFIG_VIDEO_TEGRA) += tegra-video/ obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3/ diff --git a/drivers/staging/media/starfive/Kconfig b/drivers/staging/media/starfive/Kconfig new file mode 100644 index 000000000000..34727cf56072 --- /dev/null +++ b/drivers/staging/media/starfive/Kconfig @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0-only + +comment "StarFive media platform drivers" + +source "drivers/staging/media/starfive/camss/Kconfig" diff --git a/drivers/staging/media/starfive/Makefile b/drivers/staging/media/starfive/Makefile new file mode 100644 index 000000000000..4639fa1bca32 --- /dev/null +++ b/drivers/staging/media/starfive/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-y += camss/ diff --git a/drivers/staging/media/starfive/camss/Kconfig b/drivers/staging/media/starfive/camss/Kconfig new file mode 100644 index 000000000000..8d20e2bd2559 --- /dev/null +++ b/drivers/staging/media/starfive/camss/Kconfig @@ -0,0 +1,17 @@ +# SPDX-License-Identifier: GPL-2.0-only +config VIDEO_STARFIVE_CAMSS + tristate "Starfive Camera Subsystem driver" + depends on V4L_PLATFORM_DRIVERS + depends on VIDEO_DEV && OF + depends on HAS_DMA + depends on PM + select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API + select VIDEOBUF2_DMA_CONTIG + select V4L2_FWNODE + help + Enable this to support for the Starfive Camera subsystem + found on Starfive JH7110 SoC. + + To compile this driver as a module, choose M here: the + module will be called stf-camss. diff --git a/drivers/staging/media/starfive/camss/Makefile b/drivers/staging/media/starfive/camss/Makefile new file mode 100644 index 000000000000..f53c5cbe958f --- /dev/null +++ b/drivers/staging/media/starfive/camss/Makefile @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Makefile for StarFive Camera Subsystem driver +# + +starfive-camss-objs += \ + stf_camss.o + +obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o diff --git a/drivers/staging/media/starfive/camss/stf_camss.c b/drivers/staging/media/starfive/camss/stf_camss.c new file mode 100644 index 000000000000..75ebc3a35218 --- /dev/null +++ b/drivers/staging/media/starfive/camss/stf_camss.c @@ -0,0 +1,316 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * stf_camss.c + * + * Starfive Camera Subsystem driver + * + * Copyright (C) 2021-2023 StarFive Technology Co., Ltd. + * + * Author: Jack Zhu <jack.zhu@starfivetech.com> + * Author: Changhuang Liang <changhuang.liang@starfivetech.com> + * + */ +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/videodev2.h> +#include <media/v4l2-fwnode.h> +#include <media/v4l2-mc.h> + +#include "stf_camss.h" + +static const char * const stfcamss_clocks[] = { + "clk_wrapper_clk_c", + "clk_ispcore_2x", + "clk_isp_axi", +}; + +static const char * const stfcamss_resets[] = { + "rst_wrapper_p", + "rst_wrapper_c", + "rst_axiwr", + "rst_isp_top_n", + "rst_isp_top_axi", +}; + +static int stfcamss_get_mem_res(struct stfcamss *stfcamss) +{ + struct platform_device *pdev = to_platform_device(stfcamss->dev); + + stfcamss->syscon_base = + devm_platform_ioremap_resource_byname(pdev, "syscon"); + if (IS_ERR(stfcamss->syscon_base)) + return PTR_ERR(stfcamss->syscon_base); + + stfcamss->isp_base = devm_platform_ioremap_resource_byname(pdev, "isp"); + if (IS_ERR(stfcamss->isp_base)) + return PTR_ERR(stfcamss->isp_base); + + return 0; +} + +/* + * stfcamss_of_parse_endpoint_node - Parse port endpoint node + * @dev: Device + * @node: Device node to be parsed + * @csd: Parsed data from port endpoint node + * + * Return 0 on success or a negative error code on failure + */ +static int stfcamss_of_parse_endpoint_node(struct stfcamss *stfcamss, + struct device_node *node, + struct stfcamss_async_subdev *csd) +{ + struct v4l2_fwnode_endpoint vep = { { 0 } }; + int ret; + + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(node), &vep); + if (ret) { + dev_err(stfcamss->dev, "endpoint not defined at %pOF\n", node); + return ret; + } + + csd->port = vep.base.port; + + return 0; +} + +/* + * stfcamss_of_parse_ports - Parse ports node + * @stfcamss: STFCAMSS device + * + * Return number of "port" nodes found in "ports" node + */ +static int stfcamss_of_parse_ports(struct stfcamss *stfcamss) +{ + struct device_node *node = NULL; + int ret, num_subdevs = 0; + + for_each_endpoint_of_node(stfcamss->dev->of_node, node) { + struct stfcamss_async_subdev *csd; + + if (!of_device_is_available(node)) + continue; + + csd = v4l2_async_nf_add_fwnode_remote(&stfcamss->notifier, + of_fwnode_handle(node), + struct stfcamss_async_subdev); + if (IS_ERR(csd)) { + ret = PTR_ERR(csd); + dev_err(stfcamss->dev, "failed to add async notifier\n"); + v4l2_async_nf_cleanup(&stfcamss->notifier); + return ret; + } + + ret = stfcamss_of_parse_endpoint_node(stfcamss, node, csd); + if (ret) + return ret; + + num_subdevs++; + } + + return num_subdevs; +} + +static int stfcamss_subdev_notifier_complete(struct v4l2_async_notifier *ntf) +{ + struct stfcamss *stfcamss = + container_of(ntf, struct stfcamss, notifier); + + return v4l2_device_register_subdev_nodes(&stfcamss->v4l2_dev); +} + +static const struct v4l2_async_notifier_operations +stfcamss_subdev_notifier_ops = { + .complete = stfcamss_subdev_notifier_complete, +}; + +static void stfcamss_mc_init(struct platform_device *pdev, + struct stfcamss *stfcamss) +{ + stfcamss->media_dev.dev = stfcamss->dev; + strscpy(stfcamss->media_dev.model, "Starfive Camera Subsystem", + sizeof(stfcamss->media_dev.model)); + media_device_init(&stfcamss->media_dev); + + stfcamss->v4l2_dev.mdev = &stfcamss->media_dev; +} + +/* + * stfcamss_probe - Probe STFCAMSS platform device + * @pdev: Pointer to STFCAMSS platform device + * + * Return 0 on success or a negative error code on failure + */ +static int stfcamss_probe(struct platform_device *pdev) +{ + struct stfcamss *stfcamss; + struct device *dev = &pdev->dev; + int ret, num_subdevs; + unsigned int i; + + stfcamss = devm_kzalloc(dev, sizeof(*stfcamss), GFP_KERNEL); + if (!stfcamss) + return -ENOMEM; + + stfcamss->dev = dev; + + stfcamss->nclks = ARRAY_SIZE(stfcamss->sys_clk); + for (i = 0; i < stfcamss->nclks; ++i) + stfcamss->sys_clk[i].id = stfcamss_clocks[i]; + ret = devm_clk_bulk_get(dev, stfcamss->nclks, stfcamss->sys_clk); + if (ret) { + dev_err(dev, "Failed to get clk controls\n"); + return ret; + } + + stfcamss->nrsts = ARRAY_SIZE(stfcamss->sys_rst); + for (i = 0; i < stfcamss->nrsts; ++i) + stfcamss->sys_rst[i].id = stfcamss_resets[i]; + ret = devm_reset_control_bulk_get_shared(dev, stfcamss->nrsts, + stfcamss->sys_rst); + if (ret) { + dev_err(dev, "Failed to get reset controls\n"); + return ret; + } + + ret = stfcamss_get_mem_res(stfcamss); + if (ret) { + dev_err(dev, "Could not map registers\n"); + return ret; + } + + platform_set_drvdata(pdev, stfcamss); + + v4l2_async_nf_init(&stfcamss->notifier); + + num_subdevs = stfcamss_of_parse_ports(stfcamss); + if (num_subdevs < 0) { + ret = -ENODEV; + dev_err(dev, "Failed to get sub devices: %d\n", ret); + goto err_cleanup_notifier; + } + + stfcamss_mc_init(pdev, stfcamss); + + ret = v4l2_device_register(stfcamss->dev, &stfcamss->v4l2_dev); + if (ret < 0) { + dev_err(dev, "Failed to register V4L2 device: %d\n", ret); + goto err_cleanup_media_device; + } + + ret = media_device_register(&stfcamss->media_dev); + if (ret) { + dev_err(dev, "Failed to register media device: %d\n", ret); + goto err_unregister_device; + } + + pm_runtime_enable(dev); + + stfcamss->notifier.ops = &stfcamss_subdev_notifier_ops; + ret = v4l2_async_nf_register(&stfcamss->v4l2_dev, &stfcamss->notifier); + if (ret) { + dev_err(dev, "Failed to register async subdev nodes: %d\n", + ret); + pm_runtime_disable(dev); + goto err_unregister_media_dev; + } + + return 0; + +err_unregister_media_dev: + media_device_unregister(&stfcamss->media_dev); +err_unregister_device: + v4l2_device_unregister(&stfcamss->v4l2_dev); +err_cleanup_media_device: + media_device_cleanup(&stfcamss->media_dev); +err_cleanup_notifier: + v4l2_async_nf_cleanup(&stfcamss->notifier); + return ret; +} + +/* + * stfcamss_remove - Remove STFCAMSS platform device + * @pdev: Pointer to STFCAMSS platform device + * + * Always returns 0. + */ +static int stfcamss_remove(struct platform_device *pdev) +{ + struct stfcamss *stfcamss = platform_get_drvdata(pdev); + + v4l2_device_unregister(&stfcamss->v4l2_dev); + media_device_cleanup(&stfcamss->media_dev); + pm_runtime_disable(&pdev->dev); + + return 0; +} + +static const struct of_device_id stfcamss_of_match[] = { + { .compatible = "starfive,jh7110-camss" }, + { /* sentinel */ }, +}; + +MODULE_DEVICE_TABLE(of, stfcamss_of_match); + +static int __maybe_unused stfcamss_runtime_suspend(struct device *dev) +{ + struct stfcamss *stfcamss = dev_get_drvdata(dev); + int ret; + + ret = reset_control_bulk_assert(stfcamss->nrsts, stfcamss->sys_rst); + if (ret) { + dev_err(dev, "reset assert failed\n"); + return ret; + } + + clk_bulk_disable_unprepare(stfcamss->nclks, stfcamss->sys_clk); + + return 0; +} + +static int __maybe_unused stfcamss_runtime_resume(struct device *dev) +{ + struct stfcamss *stfcamss = dev_get_drvdata(dev); + int ret; + + ret = clk_bulk_prepare_enable(stfcamss->nclks, stfcamss->sys_clk); + if (ret) { + dev_err(dev, "clock prepare enable failed\n"); + return ret; + } + + ret = reset_control_bulk_deassert(stfcamss->nrsts, stfcamss->sys_rst); + if (ret < 0) { + dev_err(dev, "cannot deassert resets\n"); + clk_bulk_disable_unprepare(stfcamss->nclks, stfcamss->sys_clk); + return ret; + } + + return 0; +} + +static const struct dev_pm_ops stfcamss_pm_ops = { + SET_RUNTIME_PM_OPS(stfcamss_runtime_suspend, + stfcamss_runtime_resume, + NULL) +}; + +static struct platform_driver stfcamss_driver = { + .probe = stfcamss_probe, + .remove = stfcamss_remove, + .driver = { + .name = "starfive-camss", + .pm = &stfcamss_pm_ops, + .of_match_table = stfcamss_of_match, + }, +}; + +module_platform_driver(stfcamss_driver); + +MODULE_AUTHOR("Jack Zhu <jack.zhu@starfivetech.com>"); +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>"); +MODULE_DESCRIPTION("StarFive Camera Subsystem driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/staging/media/starfive/camss/stf_camss.h b/drivers/staging/media/starfive/camss/stf_camss.h new file mode 100644 index 000000000000..6110433e59f7 --- /dev/null +++ b/drivers/staging/media/starfive/camss/stf_camss.h @@ -0,0 +1,129 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * stf_camss.h + * + * Starfive Camera Subsystem driver + * + * Copyright (C) 2021-2023 StarFive Technology Co., Ltd. + */ + +#ifndef STF_CAMSS_H +#define STF_CAMSS_H + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/reset.h> +#include <media/media-device.h> +#include <media/media-entity.h> +#include <media/v4l2-async.h> +#include <media/v4l2-device.h> + +enum stf_port_num { + STF_PORT_DVP = 0, + STF_PORT_CSI2RX +}; + +enum stf_clk { + STF_CLK_WRAPPER_CLK_C = 0, + STF_CLK_ISPCORE_2X, + STF_CLK_ISP_AXI, + STF_CLK_NUM +}; + +enum stf_rst { + STF_RST_WRAPPER_P = 0, + STF_RST_WRAPPER_C, + STF_RST_AXIWR, + STF_RST_ISP_TOP_N, + STF_RST_ISP_TOP_AXI, + STF_RST_NUM +}; + +struct stf_isr_data { + const char *name; + irqreturn_t (*isr)(int irq, void *priv); +}; + +struct stfcamss { + struct v4l2_device v4l2_dev; + struct media_device media_dev; + struct media_pipeline pipe; + struct device *dev; + struct v4l2_async_notifier notifier; + void __iomem *syscon_base; + void __iomem *isp_base; + struct clk_bulk_data sys_clk[STF_CLK_NUM]; + int nclks; + struct reset_control_bulk_data sys_rst[STF_RST_NUM]; + int nrsts; +}; + +struct stfcamss_async_subdev { + struct v4l2_async_subdev asd; /* must be first */ + enum stf_port_num port; +}; + +static inline u32 stf_isp_reg_read(struct stfcamss *stfcamss, u32 reg) +{ + return ioread32(stfcamss->isp_base + reg); +} + +static inline void stf_isp_reg_write(struct stfcamss *stfcamss, + u32 reg, u32 val) +{ + iowrite32(val, stfcamss->isp_base + reg); +} + +static inline void stf_isp_reg_write_delay(struct stfcamss *stfcamss, + u32 reg, u32 val, u32 delay) +{ + iowrite32(val, stfcamss->isp_base + reg); + usleep_range(1000 * delay, 1000 * delay + 100); +} + +static inline void stf_isp_reg_set_bit(struct stfcamss *stfcamss, + u32 reg, u32 mask, u32 val) +{ + u32 value; + + value = ioread32(stfcamss->isp_base + reg) & ~mask; + val &= mask; + val |= value; + iowrite32(val, stfcamss->isp_base + reg); +} + +static inline void stf_isp_reg_set(struct stfcamss *stfcamss, u32 reg, u32 mask) +{ + iowrite32(ioread32(stfcamss->isp_base + reg) | mask, + stfcamss->isp_base + reg); +} + +static inline u32 stf_syscon_reg_read(struct stfcamss *stfcamss, u32 reg) +{ + return ioread32(stfcamss->syscon_base + reg); +} + +static inline void stf_syscon_reg_write(struct stfcamss *stfcamss, + u32 reg, u32 val) +{ + iowrite32(val, stfcamss->syscon_base + reg); +} + +static inline void stf_syscon_reg_set_bit(struct stfcamss *stfcamss, + u32 reg, u32 bit_mask) +{ + u32 value; + + value = ioread32(stfcamss->syscon_base + reg); + iowrite32(value | bit_mask, stfcamss->syscon_base + reg); +} + +static inline void stf_syscon_reg_clear_bit(struct stfcamss *stfcamss, + u32 reg, u32 bit_mask) +{ + u32 value; + + value = ioread32(stfcamss->syscon_base + reg); + iowrite32(value & ~bit_mask, stfcamss->syscon_base + reg); +} +#endif /* STF_CAMSS_H */