* [v2, 1/2] ASoC: mediatek: mt6358: support DMIC one-wire mode
From: Jiaxin Yu @ 2020-06-05 9:53 UTC (permalink / raw)
To: lgirdwood, broonie, tiwai, matthias.bgg, hariprasad.kelam
Cc: alsa-devel, howie.huang, linux-kernel, Jiaxin Yu, tzungbi,
linux-mediatek, linux-arm-kernel
Supports DMIC one-wire mode. Uses a DT property "dmic-mode" to select.
Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
---
sound/soc/codecs/mt6358.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/mt6358.c b/sound/soc/codecs/mt6358.c
index 1b830ea..1f39d59 100644
--- a/sound/soc/codecs/mt6358.c
+++ b/sound/soc/codecs/mt6358.c
@@ -95,6 +95,8 @@ struct mt6358_priv {
struct regulator *avdd_reg;
int wov_enabled;
+
+ unsigned int dmic_one_wire_mode;
};
int mt6358_set_mtkaif_protocol(struct snd_soc_component *cmpnt,
@@ -1831,7 +1833,10 @@ static int mt6358_dmic_enable(struct mt6358_priv *priv)
mt6358_mtkaif_tx_enable(priv);
/* UL dmic setting */
- regmap_write(priv->regmap, MT6358_AFE_UL_SRC_CON0_H, 0x0080);
+ if (priv->dmic_one_wire_mode)
+ regmap_write(priv->regmap, MT6358_AFE_UL_SRC_CON0_H, 0x0400);
+ else
+ regmap_write(priv->regmap, MT6358_AFE_UL_SRC_CON0_H, 0x0080);
/* UL turn on */
regmap_write(priv->regmap, MT6358_AFE_UL_SRC_CON0_L, 0x0003);
@@ -2426,6 +2431,20 @@ static int mt6358_codec_probe(struct snd_soc_component *cmpnt)
.num_dapm_routes = ARRAY_SIZE(mt6358_dapm_routes),
};
+static void mt6358_parse_dt(struct mt6358_priv *priv)
+{
+ int ret;
+ struct device *dev = priv->dev;
+
+ ret = of_property_read_u32(dev->of_node, "mediatek,dmic-mode",
+ &priv->dmic_one_wire_mode);
+ if (ret) {
+ dev_warn(priv->dev, "%s() failed to read dmic-mode\n",
+ __func__);
+ priv->dmic_one_wire_mode = 0;
+ }
+}
+
static int mt6358_platform_driver_probe(struct platform_device *pdev)
{
struct mt6358_priv *priv;
@@ -2445,6 +2464,8 @@ static int mt6358_platform_driver_probe(struct platform_device *pdev)
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
+ mt6358_parse_dt(priv);
+
dev_info(priv->dev, "%s(), dev name %s\n",
__func__, dev_name(&pdev->dev));
--
1.8.1.1.dirty
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: Security Random Number Generator support
From: Russell King - ARM Linux admin @ 2020-06-05 9:27 UTC (permalink / raw)
To: Neal Liu
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Julius Werner, Herbert Xu, Arnd Bergmann, Marc Zyngier,
Matt Mackall, Sean Wang, lkml, wsd_upstream, Rob Herring,
linux-mediatek@lists.infradead.org, Linux Crypto Mailing List,
Greg Kroah-Hartman, Matthias Brugger,
Crystal Guo (郭晶), Ard Biesheuvel, Linux ARM
In-Reply-To: <1591347582.21704.9.camel@mtkswgap22>
On Fri, Jun 05, 2020 at 04:59:42PM +0800, Neal Liu wrote:
> On Fri, 2020-06-05 at 09:09 +0100, Russell King - ARM Linux admin wrote:
> > On Fri, Jun 05, 2020 at 03:19:03PM +0800, Neal Liu wrote:
> > > On Wed, 2020-06-03 at 17:34 +0800, Russell King - ARM Linux admin wrote:
> > > > This kind of thing is something that ARM have seems to shy away from
> > > > doing - it's a point I brought up many years ago when the whole
> > > > trustzone thing first appeared with its SMC call. Those around the
> > > > conference table were not interested - ARM seemed to prefer every
> > > > vendor to do off and do their own thing with the SMC interface.
> > >
> > > Does that mean it make sense to model a sec-rng driver, and get each
> > > vendor's SMC function id by DT node?
> >
> > _If_ vendors have already gone off and decided to use different SMC
> > function IDs for this, while keeping the rest of the SMC interface
> > the same, then the choice has already been made.
> >
> > I know on 32-bit that some of the secure world implementations can't
> > be changed; they're burnt into the ROM. I believe on 64-bit that isn't
> > the case, which makes it easier to standardise.
> >
> > Do you have visibility of how this SMC is implemented in the secure
> > side? Is it in ATF, and is it done as a vendor hack or is there an
> > element of generic implementation to it? Has it been submitted
> > upstream to the main ATF repository?
> >
>
> Take MediaTek as an example, some SoCs are implemented in ATF, some of
> them are implemented in TEE. We have no plan to make generic
> implementation in "secure world".
I think you have your answer right there - by _not_ making the API
generic and giving no motivation to use it, different vendors are
going to do different things (maybe even with a different API as well)
so there's no point the kernel driver pretending to be a generic
driver. If the driver isn't going to be generic, I see little point in
the SMC function number being in DT.
I think that as a _whole_ is a big mistake - there should be a generic
kernel driver for this, and there should be a standardised interface to
it through firmware. So, I would encourage you to try to get it
accepted one way or another amongst vendors as a standardised
interface.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: Security Random Number Generator support
From: Neal Liu @ 2020-06-05 8:59 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Julius Werner, Herbert Xu, Arnd Bergmann, Marc Zyngier,
Matt Mackall, Sean Wang, lkml, wsd_upstream, Rob Herring,
linux-mediatek@lists.infradead.org, Linux Crypto Mailing List,
Greg Kroah-Hartman, Matthias Brugger,
Crystal Guo (郭晶), Ard Biesheuvel, Linux ARM
In-Reply-To: <20200605080905.GF1551@shell.armlinux.org.uk>
On Fri, 2020-06-05 at 09:09 +0100, Russell King - ARM Linux admin wrote:
> On Fri, Jun 05, 2020 at 03:19:03PM +0800, Neal Liu wrote:
> > On Wed, 2020-06-03 at 17:34 +0800, Russell King - ARM Linux admin wrote:
> > > This kind of thing is something that ARM have seems to shy away from
> > > doing - it's a point I brought up many years ago when the whole
> > > trustzone thing first appeared with its SMC call. Those around the
> > > conference table were not interested - ARM seemed to prefer every
> > > vendor to do off and do their own thing with the SMC interface.
> >
> > Does that mean it make sense to model a sec-rng driver, and get each
> > vendor's SMC function id by DT node?
>
> _If_ vendors have already gone off and decided to use different SMC
> function IDs for this, while keeping the rest of the SMC interface
> the same, then the choice has already been made.
>
> I know on 32-bit that some of the secure world implementations can't
> be changed; they're burnt into the ROM. I believe on 64-bit that isn't
> the case, which makes it easier to standardise.
>
> Do you have visibility of how this SMC is implemented in the secure
> side? Is it in ATF, and is it done as a vendor hack or is there an
> element of generic implementation to it? Has it been submitted
> upstream to the main ATF repository?
>
Take MediaTek as an example, some SoCs are implemented in ATF, some of
them are implemented in TEE. We have no plan to make generic
implementation in "secure world".
Due to there must have different implementation in secure world for
vendors, we plan to provide a generic SMC interface in secure rng kernel
driver for more flexibility.
Vendors can decide which "secure world" they want (HYP/ATF/TEE) by
different smc/hvc and different SMC function IDs in DT node.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: Security Random Number Generator support
From: Russell King - ARM Linux admin @ 2020-06-05 8:09 UTC (permalink / raw)
To: Neal Liu
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Julius Werner, Herbert Xu, Arnd Bergmann, Marc Zyngier,
Matt Mackall, Sean Wang, lkml, wsd_upstream, Rob Herring,
linux-mediatek@lists.infradead.org, Linux Crypto Mailing List,
Greg Kroah-Hartman, Matthias Brugger,
Crystal Guo (郭晶), Ard Biesheuvel, Linux ARM
In-Reply-To: <1591341543.19510.4.camel@mtkswgap22>
On Fri, Jun 05, 2020 at 03:19:03PM +0800, Neal Liu wrote:
> On Wed, 2020-06-03 at 17:34 +0800, Russell King - ARM Linux admin wrote:
> > This kind of thing is something that ARM have seems to shy away from
> > doing - it's a point I brought up many years ago when the whole
> > trustzone thing first appeared with its SMC call. Those around the
> > conference table were not interested - ARM seemed to prefer every
> > vendor to do off and do their own thing with the SMC interface.
>
> Does that mean it make sense to model a sec-rng driver, and get each
> vendor's SMC function id by DT node?
_If_ vendors have already gone off and decided to use different SMC
function IDs for this, while keeping the rest of the SMC interface
the same, then the choice has already been made.
I know on 32-bit that some of the secure world implementations can't
be changed; they're burnt into the ROM. I believe on 64-bit that isn't
the case, which makes it easier to standardise.
Do you have visibility of how this SMC is implemented in the secure
side? Is it in ATF, and is it done as a vendor hack or is there an
element of generic implementation to it? Has it been submitted
upstream to the main ATF repository?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 14/14] media: platform: Add jpeg dec/enc feature
From: Xia Jiang @ 2020-06-05 8:07 UTC (permalink / raw)
To: Hans Verkuil
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, Tomasz Figa, maoguang.meng,
Matthias Brugger, sj.huang, Rob Herring, linux-mediatek,
Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
linux-media
In-Reply-To: <b62b303c-10cd-fdf6-52fa-90d63124487a@xs4all.nl>
On Mon, 2020-05-11 at 11:04 +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > decode and encode have great similarities with function operation.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8:jpeg encoder and decoder use separate callbacks instead of repeating
> > the if/else in every callback.
> > improve vidioc_try_fmt() implementation that can be shared by jpeg
> > encoder and decoder.
> > fix the bug of jpeg encoder s_selection implementation.
> > cancel the state of the jpeg encoder.
> > improve jpeg encoder and decoder set default params flow.
> > put the clock names and other datas in a match_data struct.
> > fix the bug of geting correctly quality value.
> > do the all the bits' settings of one register in one function.
> > move the code of mtk_jpeg_enc_reg.h to mtk_jpeg_enc_hw.h and delete
> > mtk_jpeg_enc_reg.h.
> >
> > v7: reverse spin lock and unlock operation in device run function for
> > multi-instance.
> >
> > v6: add space to arounding '+'.
> > alignment 'struct mtk_jpeg_fmt *fmt' match open parenthesis.
> > change 'mtk_jpeg_enc_set_encFormat' to 'mtk_jpeg_enc_set_enc_format'.
> > make 'mtk_jpeg_ctrls_setup' to static prototype.
> > delete unused variables 'jpeg'/'align_h'/'align_w'/'flags'.
> > initialize 'yuv_format'/'enc_quality' variables.
> >
> > v5: support crop for encoder and compose for decoder in s_selection and
> > g_selection function.
> > use clamp() to replace mtk_jpeg_bound_align_image() and round_up()
> > to replace mtk_jpeg_align().
> > delete jpeg_enc_param/mtk_jpeg_enc_param structure and
> > mtk_jpeg_set_param(), program the registers directly based on
> > the original V4L2 values.
> > move macro definition about hw to mtk_jpeg_enc_reg.h.
> > delete unnecessary V4L2 logs in driver.
> > cancel spin lock and unlock operation in deviec run function.
> > change jpeg enc register offset hex numberals from upercase to lowercase.
> >
> > v4: split mtk_jpeg_try_fmt_mplane() to two functions, one for encoder,
> > one for decoder.
> > split mtk_jpeg_set_default_params() to two functions, one for
> > encoder, one for decoder.
> > add cropping support for encoder in g/s_selection ioctls.
> > change exif mode support by using V4L2_JPEG_ACTIVE_MARKER_APP1.
> > change MTK_JPEG_MAX_WIDTH/MTK_JPEG_MAX_HEIGH from 8192 to 65535 by
> > specification.
> > move width shifting operation behind aligning operation in
> > mtk_jpeg_try_enc_fmt_mplane() for bug fix.
> > fix user abuseing data_offset issue for DMABUF in
> > mtk_jpeg_set_enc_src().
> > fix kbuild warings: change MTK_JPEG_MIN_HEIGHT/MTK_JPEG_MAX_HEIGHT
> > and MTK_JPEG_MIN_WIDTH/MTK_JPEG_MAX_WIDTH from
> > 'int' type to 'unsigned int' type.
> > fix msleadingly indented of 'else'.
> > v3: delete Change-Id.
> > only test once handler->error after the last v4l2_ctrl_new_std().
> > seperate changes of v4l2-ctrls.c and v4l2-controls.h to new patch.
> > v2: fix compliance test fail, check created buffer size in driver.
> > ---
> > drivers/media/platform/mtk-jpeg/Makefile | 5 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 1038 +++++++++++++----
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 51 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h | 7 +-
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 193 +++
> > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h | 123 ++
> > 6 files changed, 1188 insertions(+), 229 deletions(-)
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
> > index 48516dcf96e6..76c33aad0f3f 100644
> > --- a/drivers/media/platform/mtk-jpeg/Makefile
> > +++ b/drivers/media/platform/mtk-jpeg/Makefile
> > @@ -1,3 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_dec_hw.o mtk_jpeg_dec_parse.o
> > +mtk_jpeg-objs := mtk_jpeg_core.o \
> > + mtk_jpeg_dec_hw.o \
> > + mtk_jpeg_dec_parse.o \
> > + mtk_jpeg_enc_hw.o
> > obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 77a95185584c..18a759ce2c46 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2016 MediaTek Inc.
> > * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > * Rick Chang <rick.chang@mediatek.com>
> > + * Xia Jiang <xia.jiang@mediatek.com>
> > */
> >
> > #include <linux/clk.h>
> > @@ -23,11 +24,60 @@
> > #include <media/videobuf2-dma-contig.h>
> > #include <soc/mediatek/smi.h>
> >
> > +#include "mtk_jpeg_enc_hw.h"
> > #include "mtk_jpeg_dec_hw.h"
> > #include "mtk_jpeg_core.h"
> > #include "mtk_jpeg_dec_parse.h"
> >
> > -static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > +static struct mtk_jpeg_fmt mtk_jpeg_enc_formats[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_JPEG,
> > + .colplanes = 1,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_CAPTURE,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV12M,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_NV12,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_NV21M,
> > + .hw_format = JEPG_ENC_YUV_FORMAT_NV21,
> > + .h_sample = {4, 4},
> > + .v_sample = {4, 2},
> > + .colplanes = 2,
> > + .h_align = 4,
> > + .v_align = 4,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YUYV,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YUYV,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_YVYU,
> > + .hw_format = JPEG_ENC_YUV_FORMAT_YVYU,
> > + .h_sample = {8},
> > + .v_sample = {4},
> > + .colplanes = 1,
> > + .h_align = 5,
> > + .v_align = 3,
> > + .flags = MTK_JPEG_FMT_FLAG_ENC_OUTPUT,
> > + },
> > +};
> > +
> > +static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
> > {
> > .fourcc = V4L2_PIX_FMT_JPEG,
> > .colplanes = 1,
> > @@ -53,7 +103,8 @@ static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
> > },
> > };
> >
> > -#define MTK_JPEG_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_formats)
> > +#define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
> > +#define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
> >
> > enum {
> > MTK_JPEG_BUF_FLAGS_INIT = 0,
> > @@ -70,6 +121,11 @@ struct mtk_jpeg_src_buf {
> > static int debug;
> > module_param(debug, int, 0644);
> >
> > +static inline struct mtk_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > + return container_of(ctrl->handler, struct mtk_jpeg_ctx, ctrl_hdl);
> > +}
> > +
> > static inline struct mtk_jpeg_ctx *mtk_jpeg_fh_to_ctx(struct v4l2_fh *fh)
> > {
> > return container_of(fh, struct mtk_jpeg_ctx, fh);
> > @@ -81,12 +137,25 @@ static inline struct mtk_jpeg_src_buf *mtk_jpeg_vb2_to_srcbuf(
> > return container_of(to_vb2_v4l2_buffer(vb), struct mtk_jpeg_src_buf, b);
> > }
> >
> > -static int mtk_jpeg_querycap(struct file *file, void *priv,
> > - struct v4l2_capability *cap)
> > +static int mtk_jpeg_enc_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > +{
> > + struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > +
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > + strscpy(cap->card, MTK_JPEG_NAME " encoder", sizeof(cap->card));
> > + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > + dev_name(jpeg->dev));
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_querycap(struct file *file, void *priv,
> > + struct v4l2_capability *cap)
> > {
> > struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> >
> > - strscpy(cap->driver, MTK_JPEG_NAME " decoder", sizeof(cap->driver));
> > + strscpy(cap->driver, MTK_JPEG_NAME, sizeof(cap->driver));
> > strscpy(cap->card, MTK_JPEG_NAME " decoder", sizeof(cap->card));
> > snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> > dev_name(jpeg->dev));
> > @@ -94,6 +163,54 @@ static int mtk_jpeg_querycap(struct file *file, void *priv,
> > return 0;
> > }
> >
> > +static int vidioc_jpeg_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct mtk_jpeg_ctx *ctx = ctrl_to_ctx(ctrl);
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_JPEG_RESTART_INTERVAL:
> > + ctx->restart_interval = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_COMPRESSION_QUALITY:
> > + ctx->enc_quality = ctrl->val;
> > + break;
> > + case V4L2_CID_JPEG_ACTIVE_MARKER:
> > + ctx->enable_exif = ctrl->val & V4L2_JPEG_ACTIVE_MARKER_APP1 ?
> > + true : false;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mtk_jpeg_enc_ctrl_ops = {
> > + .s_ctrl = vidioc_jpeg_enc_s_ctrl,
> > +};
> > +
> > +static int mtk_jpeg_enc_ctrls_setup(struct mtk_jpeg_ctx *ctx)
> > +{
> > + const struct v4l2_ctrl_ops *ops = &mtk_jpeg_enc_ctrl_ops;
> > + struct v4l2_ctrl_handler *handler = &ctx->ctrl_hdl;
> > +
> > + v4l2_ctrl_handler_init(handler, 3);
> > +
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_RESTART_INTERVAL, 0, 100,
> > + 1, 0);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_COMPRESSION_QUALITY, 48,
> > + 100, 1, 90);
> > + v4l2_ctrl_new_std(handler, ops, V4L2_CID_JPEG_ACTIVE_MARKER, 0,
> > + V4L2_JPEG_ACTIVE_MARKER_APP1, 0, 0);
> > +
> > + if (handler->error) {
> > + v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > + return handler->error;
> > + }
> > +
> > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > +
> > + return 0;
> > +}
> > +
> > static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > struct v4l2_fmtdesc *f, u32 type)
> > {
> > @@ -115,117 +232,105 @@ static int mtk_jpeg_enum_fmt(struct mtk_jpeg_fmt *mtk_jpeg_formats, int n,
> > return 0;
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_cap(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats,
> > + MTK_JPEG_DEC_NUM_FORMATS, f,
> > MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > }
> >
> > -static int mtk_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
> > - struct v4l2_fmtdesc *f)
> > +static int mtk_jpeg_enc_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > +{
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_enc_formats,
> > + MTK_JPEG_ENC_NUM_FORMATS, f,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_enum_fmt_vid_out(struct file *file, void *priv,
> > + struct v4l2_fmtdesc *f)
> > {
> > - return mtk_jpeg_enum_fmt(mtk_jpeg_formats, MTK_JPEG_NUM_FORMATS, f,
> > - MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > + return mtk_jpeg_enum_fmt(mtk_jpeg_dec_formats, MTK_JPEG_DEC_NUM_FORMATS,
> > + f, MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > }
>
> OK, so this patch is very hard to read because there are two independent changes
> taking place:
>
> 1) rename existing functions/defines/variables with a _dec prefix to prepare
> for the addition of the encoder feature.
>
> 2) add the encoder feature.
>
> Please split up this patch into two parts: one that does the rename and as much of
> the preparation to support both decoder and encoder without changing the
> functionality, and a second one that actually adds the new encoder feature.
>
> In fact, once that's done it is likely that most of this patch series can be
> merged, even if there are still things that need to be changed for the last
> patch adding the encoder support. I see nothing objectionable in patches 1-10
> and 13. So merging those together with a new rename patch wouldn't be an issue,
> I think.
>
> In any case, the diffs should be a lot cleaner and easier to review by splitting
> it up like that.
Dear Hans,
Thanks for your good advice. I have splited up this patch into two
patches in v9.
Best Regards,
Xia Jiang
>
> Regards,
>
> Hans
>
> >
> > -static struct mtk_jpeg_q_data *mtk_jpeg_get_q_data(struct mtk_jpeg_ctx *ctx,
> > - enum v4l2_buf_type type)
> > +static struct mtk_jpeg_q_data *
> > +mtk_jpeg_get_q_data(struct mtk_jpeg_ctx *ctx, enum v4l2_buf_type type)
> > {
> > if (V4L2_TYPE_IS_OUTPUT(type))
> > return &ctx->out_q;
> > return &ctx->cap_q;
> > }
> >
> > -static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> > - u32 pixelformat,
> > +static struct mtk_jpeg_fmt *mtk_jpeg_find_format(u32 pixelformat,
> > unsigned int fmt_type)
> > {
> > - unsigned int k, fmt_flag;
> > -
> > - fmt_flag = (fmt_type == MTK_JPEG_FMT_TYPE_OUTPUT) ?
> > - MTK_JPEG_FMT_FLAG_DEC_OUTPUT :
> > - MTK_JPEG_FMT_FLAG_DEC_CAPTURE;
> > + unsigned int k;
> > + struct mtk_jpeg_fmt *fmt;
> >
> > - for (k = 0; k < MTK_JPEG_NUM_FORMATS; k++) {
> > - struct mtk_jpeg_fmt *fmt = &mtk_jpeg_formats[k];
> > + for (k = 0; k < MTK_JPEG_ENC_NUM_FORMATS; k++) {
> > + fmt = &mtk_jpeg_enc_formats[k];
> >
> > - if (fmt->fourcc == pixelformat && fmt->flags & fmt_flag)
> > + if (fmt->fourcc == pixelformat && fmt->flags & fmt_type)
> > return fmt;
> > }
> >
> > - return NULL;
> > -}
> > + for (k = 0; k < MTK_JPEG_DEC_NUM_FORMATS; k++) {
> > + fmt = &mtk_jpeg_dec_formats[k];
> >
> > -static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > - struct v4l2_format *f)
> > -{
> > - struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > - struct mtk_jpeg_q_data *q_data;
> > - int i;
> > -
> > - q_data = mtk_jpeg_get_q_data(ctx, f->type);
> > -
> > - pix_mp->width = q_data->w;
> > - pix_mp->height = q_data->h;
> > - pix_mp->pixelformat = q_data->fmt->fourcc;
> > - pix_mp->num_planes = q_data->fmt->colplanes;
> > -
> > - for (i = 0; i < pix_mp->num_planes; i++) {
> > - pix_mp->plane_fmt[i].bytesperline = q_data->bytesperline[i];
> > - pix_mp->plane_fmt[i].sizeimage = q_data->sizeimage[i];
> > + if (fmt->fourcc == pixelformat && fmt->flags & fmt_type)
> > + return fmt;
> > }
> > +
> > + return NULL;
> > }
> >
> > -static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > - struct mtk_jpeg_fmt *fmt,
> > - struct mtk_jpeg_ctx *ctx, int q_type)
> > +static int vidioc_try_fmt(struct v4l2_format *f, struct mtk_jpeg_fmt *fmt)
> > {
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > int i;
> >
> > - memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > pix_mp->field = V4L2_FIELD_NONE;
> > -
> > - if (ctx->state != MTK_JPEG_INIT) {
> > - mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > - return 0;
> > - }
> > -
> > pix_mp->num_planes = fmt->colplanes;
> > pix_mp->pixelformat = fmt->fourcc;
> >
> > - if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> > - struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> > -
> > + if (fmt->fourcc == V4L2_PIX_FMT_JPEG) {
> > pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > MTK_JPEG_MAX_HEIGHT);
> > pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > MTK_JPEG_MAX_WIDTH);
> > -
> > - memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > - pfmt->bytesperline = 0;
> > - /* Source size must be aligned to 128 */
> > - pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > - if (pfmt->sizeimage == 0)
> > - pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > - return 0;
> > + pix_mp->plane_fmt[0].bytesperline = 0;
> > + pix_mp->plane_fmt[0].sizeimage =
> > + round_up(pix_mp->plane_fmt[0].sizeimage, 128);
> > + if (pix_mp->plane_fmt[0].sizeimage == 0)
> > + pix_mp->plane_fmt[0].sizeimage =
> > + MTK_JPEG_DEFAULT_SIZEIMAGE;
> > + } else {
> > + pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > + MTK_JPEG_MIN_HEIGHT,
> > + MTK_JPEG_MAX_HEIGHT);
> > + pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > + MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> > + for (i = 0; i < pix_mp->num_planes; i++) {
> > + struct v4l2_plane_pix_format *pfmt =
> > + &pix_mp->plane_fmt[i];
> > + u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > + u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> > +
> > + pfmt->bytesperline = stride;
> > + pfmt->sizeimage = stride * h;
> > + }
> > }
> >
> > - /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > - pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > - MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> > - pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > - MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> > -
> > - for (i = 0; i < fmt->colplanes; i++) {
> > - struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > - u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > - u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> > -
> > - pfmt->bytesperline = stride;
> > - pfmt->sizeimage = stride * h;
> > - }
> > return 0;
> > }
> >
> > @@ -280,14 +385,35 @@ static int mtk_jpeg_g_fmt_vid_mplane(struct file *file, void *priv,
> > return 0;
> > }
> >
> > -static int mtk_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > - struct v4l2_format *f)
> > +static int mtk_jpeg_enc_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > +{
> > + struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > + struct mtk_jpeg_fmt *fmt;
> > +
> > + fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > + if (!fmt)
> > + fmt = ctx->cap_q.fmt;
> > +
> > + v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%c%c%c%c\n",
> > + f->type,
> > + (fmt->fourcc & 0xff),
> > + (fmt->fourcc >> 8 & 0xff),
> > + (fmt->fourcc >> 16 & 0xff),
> > + (fmt->fourcc >> 24 & 0xff));
> > +
> > + return vidioc_try_fmt(f, fmt);
> > +}
> > +
> > +static int mtk_jpeg_dec_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > {
> > struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > struct mtk_jpeg_fmt *fmt;
> >
> > - fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > - MTK_JPEG_FMT_TYPE_CAPTURE);
> > + fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > + MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > if (!fmt)
> > fmt = ctx->cap_q.fmt;
> >
> > @@ -298,17 +424,43 @@ static int mtk_jpeg_try_fmt_vid_cap_mplane(struct file *file, void *priv,
> > (fmt->fourcc >> 16 & 0xff),
> > (fmt->fourcc >> 24 & 0xff));
> >
> > - return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_CAPTURE);
> > + if (ctx->state != MTK_JPEG_INIT) {
> > + mtk_jpeg_g_fmt_vid_mplane(file, priv, f);
> > + return 0;
> > + }
> > +
> > + return vidioc_try_fmt(f, fmt);
> > +}
> > +
> > +static int mtk_jpeg_enc_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > +{
> > + struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > + struct mtk_jpeg_fmt *fmt;
> > +
> > + fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > + if (!fmt)
> > + fmt = ctx->out_q.fmt;
> > +
> > + v4l2_dbg(2, debug, &ctx->jpeg->v4l2_dev, "(%d) try_fmt:%c%c%c%c\n",
> > + f->type,
> > + (fmt->fourcc & 0xff),
> > + (fmt->fourcc >> 8 & 0xff),
> > + (fmt->fourcc >> 16 & 0xff),
> > + (fmt->fourcc >> 24 & 0xff));
> > +
> > + return vidioc_try_fmt(f, fmt);
> > }
> >
> > -static int mtk_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > - struct v4l2_format *f)
> > +static int mtk_jpeg_dec_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > {
> > struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > struct mtk_jpeg_fmt *fmt;
> >
> > - fmt = mtk_jpeg_find_format(ctx, f->fmt.pix_mp.pixelformat,
> > - MTK_JPEG_FMT_TYPE_OUTPUT);
> > + fmt = mtk_jpeg_find_format(f->fmt.pix_mp.pixelformat,
> > + MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > if (!fmt)
> > fmt = ctx->out_q.fmt;
> >
> > @@ -319,17 +471,21 @@ static int mtk_jpeg_try_fmt_vid_out_mplane(struct file *file, void *priv,
> > (fmt->fourcc >> 16 & 0xff),
> > (fmt->fourcc >> 24 & 0xff));
> >
> > - return mtk_jpeg_try_fmt_mplane(f, fmt, ctx, MTK_JPEG_FMT_TYPE_OUTPUT);
> > + if (ctx->state != MTK_JPEG_INIT) {
> > + mtk_jpeg_g_fmt_vid_mplane(file, priv, f);
> > + return 0;
> > + }
> > +
> > + return vidioc_try_fmt(f, fmt);
> > }
> >
> > static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > - struct v4l2_format *f)
> > + struct v4l2_format *f, unsigned int fmt_type)
> > {
> > struct vb2_queue *vq;
> > struct mtk_jpeg_q_data *q_data = NULL;
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > - unsigned int f_type;
> > int i;
> >
> > vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > @@ -343,10 +499,7 @@ static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > return -EBUSY;
> > }
> >
> > - f_type = V4L2_TYPE_IS_OUTPUT(f->type) ?
> > - MTK_JPEG_FMT_TYPE_OUTPUT : MTK_JPEG_FMT_TYPE_CAPTURE;
> > -
> > - q_data->fmt = mtk_jpeg_find_format(ctx, pix_mp->pixelformat, f_type);
> > + q_data->fmt = mtk_jpeg_find_format(pix_mp->pixelformat, fmt_type);
> > q_data->w = pix_mp->width;
> > q_data->h = pix_mp->height;
> > ctx->colorspace = pix_mp->colorspace;
> > @@ -374,28 +527,56 @@ static int mtk_jpeg_s_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > return 0;
> > }
> >
> > -static int mtk_jpeg_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > - struct v4l2_format *f)
> > +static int mtk_jpeg_enc_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > +{
> > + int ret;
> > +
> > + ret = mtk_jpeg_enc_try_fmt_vid_out_mplane(file, priv, f);
> > + if (ret)
> > + return ret;
> > +
> > + return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > +}
> > +
> > +static int mtk_jpeg_dec_s_fmt_vid_out_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > {
> > int ret;
> >
> > - ret = mtk_jpeg_try_fmt_vid_out_mplane(file, priv, f);
> > + ret = mtk_jpeg_dec_try_fmt_vid_out_mplane(file, priv, f);
> > if (ret)
> > return ret;
> >
> > - return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > + return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > + MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > }
> >
> > -static int mtk_jpeg_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > - struct v4l2_format *f)
> > +static int mtk_jpeg_enc_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > {
> > int ret;
> >
> > - ret = mtk_jpeg_try_fmt_vid_cap_mplane(file, priv, f);
> > + ret = mtk_jpeg_enc_try_fmt_vid_cap_mplane(file, priv, f);
> > if (ret)
> > return ret;
> >
> > - return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f);
> > + return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > +}
> > +
> > +static int mtk_jpeg_dec_s_fmt_vid_cap_mplane(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > +{
> > + int ret;
> > +
> > + ret = mtk_jpeg_dec_try_fmt_vid_cap_mplane(file, priv, f);
> > + if (ret)
> > + return ret;
> > +
> > + return mtk_jpeg_s_fmt_mplane(mtk_jpeg_fh_to_ctx(priv), f,
> > + MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > }
> >
> > static void mtk_jpeg_queue_src_chg_event(struct mtk_jpeg_ctx *ctx)
> > @@ -420,8 +601,31 @@ static int mtk_jpeg_subscribe_event(struct v4l2_fh *fh,
> > return v4l2_ctrl_subscribe_event(fh, sub);
> > }
> >
> > -static int mtk_jpeg_g_selection(struct file *file, void *priv,
> > - struct v4l2_selection *s)
> > +static int mtk_jpeg_enc_g_selection(struct file *file, void *priv,
> > + struct v4l2_selection *s)
> > +{
> > + struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > + return -EINVAL;
> > +
> > + switch (s->target) {
> > + case V4L2_SEL_TGT_CROP:
> > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > + case V4L2_SEL_TGT_CROP_DEFAULT:
> > + s->r.width = ctx->out_q.w;
> > + s->r.height = ctx->out_q.h;
> > + s->r.left = 0;
> > + s->r.top = 0;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_g_selection(struct file *file, void *priv,
> > + struct v4l2_selection *s)
> > {
> > struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >
> > @@ -446,11 +650,34 @@ static int mtk_jpeg_g_selection(struct file *file, void *priv,
> > default:
> > return -EINVAL;
> > }
> > +
> > return 0;
> > }
> >
> > -static int mtk_jpeg_s_selection(struct file *file, void *priv,
> > - struct v4l2_selection *s)
> > +static int mtk_jpeg_enc_s_selection(struct file *file, void *priv,
> > + struct v4l2_selection *s)
> > +{
> > + struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> > +
> > + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > + return -EINVAL;
> > +
> > + switch (s->target) {
> > + case V4L2_SEL_TGT_CROP:
> > + s->r.left = 0;
> > + s->r.top = 0;
> > + ctx->out_q.w = min(s->r.width, ctx->out_q.w);
> > + ctx->out_q.h = min(s->r.height, ctx->out_q.h);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mtk_jpeg_dec_s_selection(struct file *file, void *priv,
> > + struct v4l2_selection *s)
> > {
> > struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
> >
> > @@ -467,6 +694,7 @@ static int mtk_jpeg_s_selection(struct file *file, void *priv,
> > default:
> > return -EINVAL;
> > }
> > +
> > return 0;
> > }
> >
> > @@ -495,20 +723,47 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> > return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> > }
> >
> > -static const struct v4l2_ioctl_ops mtk_jpeg_ioctl_ops = {
> > - .vidioc_querycap = mtk_jpeg_querycap,
> > - .vidioc_enum_fmt_vid_cap = mtk_jpeg_enum_fmt_vid_cap,
> > - .vidioc_enum_fmt_vid_out = mtk_jpeg_enum_fmt_vid_out,
> > - .vidioc_try_fmt_vid_cap_mplane = mtk_jpeg_try_fmt_vid_cap_mplane,
> > - .vidioc_try_fmt_vid_out_mplane = mtk_jpeg_try_fmt_vid_out_mplane,
> > +static const struct v4l2_ioctl_ops mtk_jpeg_enc_ioctl_ops = {
> > + .vidioc_querycap = mtk_jpeg_enc_querycap,
> > + .vidioc_enum_fmt_vid_cap = mtk_jpeg_enc_enum_fmt_vid_cap,
> > + .vidioc_enum_fmt_vid_out = mtk_jpeg_enc_enum_fmt_vid_out,
> > + .vidioc_try_fmt_vid_cap_mplane = mtk_jpeg_enc_try_fmt_vid_cap_mplane,
> > + .vidioc_try_fmt_vid_out_mplane = mtk_jpeg_enc_try_fmt_vid_out_mplane,
> > + .vidioc_g_fmt_vid_cap_mplane = mtk_jpeg_g_fmt_vid_mplane,
> > + .vidioc_g_fmt_vid_out_mplane = mtk_jpeg_g_fmt_vid_mplane,
> > + .vidioc_s_fmt_vid_cap_mplane = mtk_jpeg_enc_s_fmt_vid_cap_mplane,
> > + .vidioc_s_fmt_vid_out_mplane = mtk_jpeg_enc_s_fmt_vid_out_mplane,
> > + .vidioc_qbuf = mtk_jpeg_qbuf,
> > + .vidioc_subscribe_event = mtk_jpeg_subscribe_event,
> > + .vidioc_g_selection = mtk_jpeg_enc_g_selection,
> > + .vidioc_s_selection = mtk_jpeg_enc_s_selection,
> > +
> > + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> > + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> > + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> > +
> > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> > +};
> > +
> > +static const struct v4l2_ioctl_ops mtk_jpeg_dec_ioctl_ops = {
> > + .vidioc_querycap = mtk_jpeg_dec_querycap,
> > + .vidioc_enum_fmt_vid_cap = mtk_jpeg_dec_enum_fmt_vid_cap,
> > + .vidioc_enum_fmt_vid_out = mtk_jpeg_dec_enum_fmt_vid_out,
> > + .vidioc_try_fmt_vid_cap_mplane = mtk_jpeg_dec_try_fmt_vid_cap_mplane,
> > + .vidioc_try_fmt_vid_out_mplane = mtk_jpeg_dec_try_fmt_vid_out_mplane,
> > .vidioc_g_fmt_vid_cap_mplane = mtk_jpeg_g_fmt_vid_mplane,
> > .vidioc_g_fmt_vid_out_mplane = mtk_jpeg_g_fmt_vid_mplane,
> > - .vidioc_s_fmt_vid_cap_mplane = mtk_jpeg_s_fmt_vid_cap_mplane,
> > - .vidioc_s_fmt_vid_out_mplane = mtk_jpeg_s_fmt_vid_out_mplane,
> > + .vidioc_s_fmt_vid_cap_mplane = mtk_jpeg_dec_s_fmt_vid_cap_mplane,
> > + .vidioc_s_fmt_vid_out_mplane = mtk_jpeg_dec_s_fmt_vid_out_mplane,
> > .vidioc_qbuf = mtk_jpeg_qbuf,
> > .vidioc_subscribe_event = mtk_jpeg_subscribe_event,
> > - .vidioc_g_selection = mtk_jpeg_g_selection,
> > - .vidioc_s_selection = mtk_jpeg_s_selection,
> > + .vidioc_g_selection = mtk_jpeg_dec_g_selection,
> > + .vidioc_s_selection = mtk_jpeg_dec_s_selection,
> >
> > .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> > .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> > @@ -586,8 +841,9 @@ static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx,
> > }
> >
> > q_data = &ctx->cap_q;
> > - if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> > - MTK_JPEG_FMT_TYPE_CAPTURE)) {
> > + if (q_data->fmt !=
> > + mtk_jpeg_find_format(param->dst_fourcc,
> > + MTK_JPEG_FMT_FLAG_DEC_CAPTURE)) {
> > v4l2_dbg(1, debug, &jpeg->v4l2_dev, "format change\n");
> > return true;
> > }
> > @@ -608,9 +864,8 @@ static void mtk_jpeg_set_queue_data(struct mtk_jpeg_ctx *ctx,
> > q_data = &ctx->cap_q;
> > q_data->w = param->dec_w;
> > q_data->h = param->dec_h;
> > - q_data->fmt = mtk_jpeg_find_format(ctx,
> > - param->dst_fourcc,
> > - MTK_JPEG_FMT_TYPE_CAPTURE);
> > + q_data->fmt = mtk_jpeg_find_format(param->dst_fourcc,
> > + MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> >
> > for (i = 0; i < q_data->fmt->colplanes; i++) {
> > q_data->bytesperline[i] = param->mem_stride[i];
> > @@ -627,7 +882,18 @@ static void mtk_jpeg_set_queue_data(struct mtk_jpeg_ctx *ctx,
> > param->dec_w, param->dec_h);
> > }
> >
> > -static void mtk_jpeg_buf_queue(struct vb2_buffer *vb)
> > +static void mtk_jpeg_enc_buf_queue(struct vb2_buffer *vb)
> > +{
> > + struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +
> > + v4l2_dbg(2, debug, &jpeg->v4l2_dev, "(%d) buf_q id=%d, vb=%p\n",
> > + vb->vb2_queue->type, vb->index, vb);
> > +
> > + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, to_vb2_v4l2_buffer(vb));
> > +}
> > +
> > +static void mtk_jpeg_dec_buf_queue(struct vb2_buffer *vb)
> > {
> > struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > struct mtk_jpeg_dec_param *param;
> > @@ -679,7 +945,16 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> > return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > }
> >
> > -static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > +static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
> > +{
> > + struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > + struct vb2_v4l2_buffer *vb;
> > +
> > + while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > + v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > +}
> > +
> > +static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
> > {
> > struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > struct vb2_v4l2_buffer *vb;
> > @@ -705,13 +980,22 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > }
> >
> > -static const struct vb2_ops mtk_jpeg_qops = {
> > +static const struct vb2_ops mtk_jpeg_dec_qops = {
> > .queue_setup = mtk_jpeg_queue_setup,
> > .buf_prepare = mtk_jpeg_buf_prepare,
> > - .buf_queue = mtk_jpeg_buf_queue,
> > + .buf_queue = mtk_jpeg_dec_buf_queue,
> > .wait_prepare = vb2_ops_wait_prepare,
> > .wait_finish = vb2_ops_wait_finish,
> > - .stop_streaming = mtk_jpeg_stop_streaming,
> > + .stop_streaming = mtk_jpeg_dec_stop_streaming,
> > +};
> > +
> > +static const struct vb2_ops mtk_jpeg_enc_qops = {
> > + .queue_setup = mtk_jpeg_queue_setup,
> > + .buf_prepare = mtk_jpeg_buf_prepare,
> > + .buf_queue = mtk_jpeg_enc_buf_queue,
> > + .wait_prepare = vb2_ops_wait_prepare,
> > + .wait_finish = vb2_ops_wait_finish,
> > + .stop_streaming = mtk_jpeg_enc_stop_streaming,
> > };
> >
> > static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> > @@ -751,7 +1035,86 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > return 0;
> > }
> >
> > -static void mtk_jpeg_device_run(void *priv)
> > +static void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base,
> > + struct vb2_buffer *dst_buf,
> > + struct mtk_jpeg_enc_bs *bs)
> > +{
> > + bs->dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> > + bs->dma_addr_offset = ctx->enable_exif ? MTK_JPEG_DEFAULT_EXIF_SIZE : 0;
> > + bs->dma_addr_offsetmask = bs->dma_addr & JPEG_ENC_DST_ADDR_OFFSET_MASK;
> > + bs->size = vb2_plane_size(dst_buf, 0);
> > +
> > + mtk_jpeg_enc_set_dst_addr(base, bs->dma_addr, bs->size,
> > + bs->dma_addr_offset,
> > + bs->dma_addr_offsetmask);
> > +}
> > +
> > +static void mtk_jpeg_set_enc_src(struct mtk_jpeg_ctx *ctx, void __iomem *base,
> > + struct vb2_buffer *src_buf)
> > +{
> > + int i;
> > + dma_addr_t dma_addr;
> > +
> > + mtk_jpeg_enc_set_img_size(base, ctx->out_q.w, ctx->out_q.h);
> > + mtk_jpeg_enc_set_blk_num(base, ctx->out_q.fmt->fourcc, ctx->out_q.w,
> > + ctx->out_q.h);
> > + mtk_jpeg_enc_set_stride(base, ctx->out_q.fmt->fourcc, ctx->out_q.w,
> > + ctx->out_q.h, ctx->out_q.bytesperline[0]);
> > +
> > + for (i = 0; i < src_buf->num_planes; i++) {
> > + dma_addr = vb2_dma_contig_plane_dma_addr(src_buf, i) +
> > + src_buf->planes[i].data_offset;
> > + mtk_jpeg_enc_set_src_addr(base, dma_addr, i);
> > + }
> > +}
> > +
> > +static void mtk_jpeg_enc_device_run(void *priv)
> > +{
> > + struct mtk_jpeg_ctx *ctx = priv;
> > + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > + enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > + unsigned long flags;
> > + struct mtk_jpeg_src_buf *jpeg_src_buf;
> > + struct mtk_jpeg_enc_bs enc_bs;
> > + int i, ret;
> > +
> > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > + jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> > +
> > + if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> > + for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
> > + vb2_set_plane_payload(&dst_buf->vb2_buf, i, 0);
> > + buf_state = VB2_BUF_STATE_DONE;
> > + goto enc_end;
> > + }
> > +
> > + ret = pm_runtime_get_sync(jpeg->dev);
> > + if (ret < 0)
> > + goto enc_end;
> > +
> > + spin_lock_irqsave(&jpeg->hw_lock, flags);
> > + mtk_jpeg_enc_reset(jpeg->reg_base);
> > +
> > + mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf, &enc_bs);
> > + mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf);
> > + mtk_jpeg_enc_set_config(jpeg->reg_base, ctx->out_q.fmt->hw_format,
> > + ctx->enable_exif, ctx->enc_quality,
> > + ctx->restart_interval);
> > + mtk_jpeg_enc_start(jpeg->reg_base);
> > + spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > + return;
> > +
> > +enc_end:
> > + v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > + v4l2_m2m_buf_done(src_buf, buf_state);
> > + v4l2_m2m_buf_done(dst_buf, buf_state);
> > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +}
> > +
> > +static void mtk_jpeg_dec_device_run(void *priv)
> > {
> > struct mtk_jpeg_ctx *ctx = priv;
> > struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > @@ -786,15 +1149,16 @@ static void mtk_jpeg_device_run(void *priv)
> > goto dec_end;
> >
> > mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> > - if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> > + if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param,
> > + &dst_buf->vb2_buf, &fb))
> > goto dec_end;
> >
> > spin_lock_irqsave(&jpeg->hw_lock, flags);
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > - mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> > + mtk_jpeg_dec_reset(jpeg->reg_base);
> > + mtk_jpeg_dec_set_config(jpeg->reg_base,
> > &jpeg_src_buf->dec_param, &bs, &fb);
> >
> > - mtk_jpeg_dec_start(jpeg->dec_reg_base);
> > + mtk_jpeg_dec_start(jpeg->reg_base);
> > spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > return;
> >
> > @@ -806,20 +1170,30 @@ static void mtk_jpeg_device_run(void *priv)
> > v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > }
> >
> > -static int mtk_jpeg_job_ready(void *priv)
> > +static int mtk_jpeg_enc_job_ready(void *priv)
> > +{
> > + return 1;
> > +}
> > +
> > +static int mtk_jpeg_dec_job_ready(void *priv)
> > {
> > struct mtk_jpeg_ctx *ctx = priv;
> >
> > return (ctx->state == MTK_JPEG_RUNNING) ? 1 : 0;
> > }
> >
> > -static const struct v4l2_m2m_ops mtk_jpeg_m2m_ops = {
> > - .device_run = mtk_jpeg_device_run,
> > - .job_ready = mtk_jpeg_job_ready,
> > +static const struct v4l2_m2m_ops mtk_jpeg_enc_m2m_ops = {
> > + .device_run = mtk_jpeg_enc_device_run,
> > + .job_ready = mtk_jpeg_enc_job_ready,
> > };
> >
> > -static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > - struct vb2_queue *dst_vq)
> > +static const struct v4l2_m2m_ops mtk_jpeg_dec_m2m_ops = {
> > + .device_run = mtk_jpeg_dec_device_run,
> > + .job_ready = mtk_jpeg_dec_job_ready,
> > +};
> > +
> > +static int mtk_jpeg_dec_queue_init(void *priv, struct vb2_queue *src_vq,
> > + struct vb2_queue *dst_vq)
> > {
> > struct mtk_jpeg_ctx *ctx = priv;
> > int ret;
> > @@ -828,7 +1202,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > src_vq->drv_priv = ctx;
> > src_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
> > - src_vq->ops = &mtk_jpeg_qops;
> > + src_vq->ops = &mtk_jpeg_dec_qops;
> > src_vq->mem_ops = &vb2_dma_contig_memops;
> > src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > src_vq->lock = &ctx->jpeg->lock;
> > @@ -841,7 +1215,7 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > dst_vq->drv_priv = ctx;
> > dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > - dst_vq->ops = &mtk_jpeg_qops;
> > + dst_vq->ops = &mtk_jpeg_dec_qops;
> > dst_vq->mem_ops = &vb2_dma_contig_memops;
> > dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > dst_vq->lock = &ctx->jpeg->lock;
> > @@ -851,24 +1225,112 @@ static int mtk_jpeg_queue_init(void *priv, struct vb2_queue *src_vq,
> > return ret;
> > }
> >
> > -static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > +static int mtk_jpeg_enc_queue_init(void *priv, struct vb2_queue *src_vq,
> > + struct vb2_queue *dst_vq)
> > {
> > + struct mtk_jpeg_ctx *ctx = priv;
> > int ret;
> >
> > + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > + src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > + src_vq->drv_priv = ctx;
> > + src_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
> > + src_vq->ops = &mtk_jpeg_enc_qops;
> > + src_vq->mem_ops = &vb2_dma_contig_memops;
> > + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > + src_vq->lock = &ctx->jpeg->lock;
> > + src_vq->dev = ctx->jpeg->dev;
> > + ret = vb2_queue_init(src_vq);
> > + if (ret)
> > + return ret;
> > +
> > + dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > + dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> > + dst_vq->drv_priv = ctx;
> > + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > + dst_vq->ops = &mtk_jpeg_enc_qops;
> > + dst_vq->mem_ops = &vb2_dma_contig_memops;
> > + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > + dst_vq->lock = &ctx->jpeg->lock;
> > + dst_vq->dev = ctx->jpeg->dev;
> > + ret = vb2_queue_init(dst_vq);
> > +
> > + return ret;
> > +}
> > +
> > +static void mtk_jpeg_clk_on(struct mtk_jpeg_dev *jpeg)
> > +{
> > + int ret, i;
> > +
> > ret = mtk_smi_larb_get(jpeg->larb);
> > if (ret)
> > dev_err(jpeg->dev, "mtk_smi_larb_get larbvdec fail %d\n", ret);
> > - clk_prepare_enable(jpeg->clk_jdec_smi);
> > - clk_prepare_enable(jpeg->clk_jdec);
> > +
> > + for (i = 0; i < jpeg->variant->num_clocks; i++) {
> > + ret = clk_prepare_enable(jpeg->clocks[i]);
> > + if (ret) {
> > + while (--i >= 0)
> > + clk_disable_unprepare(jpeg->clocks[i]);
> > + }
> > + }
> > }
> >
> > static void mtk_jpeg_clk_off(struct mtk_jpeg_dev *jpeg)
> > {
> > - clk_disable_unprepare(jpeg->clk_jdec);
> > - clk_disable_unprepare(jpeg->clk_jdec_smi);
> > + int i;
> > +
> > + for (i = jpeg->variant->num_clocks - 1; i >= 0; i--)
> > + clk_disable_unprepare(jpeg->clocks[i]);
> > mtk_smi_larb_put(jpeg->larb);
> > }
> >
> > +static irqreturn_t mtk_jpeg_enc_irq(int irq, void *priv)
> > +{
> > + struct mtk_jpeg_dev *jpeg = priv;
> > + struct mtk_jpeg_ctx *ctx;
> > + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > + struct mtk_jpeg_src_buf *jpeg_src_buf;
> > + enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > + u32 enc_irq_ret;
> > + u32 enc_ret, result_size;
> > +
> > + spin_lock(&jpeg->hw_lock);
> > +
> > + ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > + if (!ctx) {
> > + v4l2_err(&jpeg->v4l2_dev, "Context is NULL\n");
> > + return IRQ_HANDLED;
> > + }
> > +
> > + src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > + jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> > +
> > + enc_ret = mtk_jpeg_enc_get_and_clear_int_status(jpeg->reg_base);
> > + enc_irq_ret = mtk_jpeg_enc_enum_result(jpeg->reg_base, enc_ret);
> > +
> > + if (enc_irq_ret >= MTK_JPEG_ENC_RESULT_STALL)
> > + mtk_jpeg_enc_reset(jpeg->reg_base);
> > +
> > + if (enc_irq_ret != MTK_JPEG_ENC_RESULT_DONE) {
> > + dev_err(jpeg->dev, "encode failed\n");
> > + goto enc_end;
> > + }
> > +
> > + result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base);
> > + vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
> > +
> > + buf_state = VB2_BUF_STATE_DONE;
> > +
> > +enc_end:
> > + v4l2_m2m_buf_done(src_buf, buf_state);
> > + v4l2_m2m_buf_done(dst_buf, buf_state);
> > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + spin_unlock(&jpeg->hw_lock);
> > + pm_runtime_put_sync(ctx->jpeg->dev);
> > + return IRQ_HANDLED;
> > +}
> > +
> > static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > {
> > struct mtk_jpeg_dev *jpeg = priv;
> > @@ -876,13 +1338,13 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > struct mtk_jpeg_src_buf *jpeg_src_buf;
> > enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > - u32 dec_irq_ret;
> > + u32 dec_irq_ret;
> > u32 dec_ret;
> > int i;
> >
> > spin_lock(&jpeg->hw_lock);
> >
> > - dec_ret = mtk_jpeg_dec_get_int_status(jpeg->dec_reg_base);
> > + dec_ret = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
> > dec_irq_ret = mtk_jpeg_dec_enum_result(dec_ret);
> > ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
> > if (!ctx) {
> > @@ -895,7 +1357,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> >
> > if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
> > - mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > + mtk_jpeg_dec_reset(jpeg->reg_base);
> >
> > if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
> > dev_err(jpeg->dev, "decode failed\n");
> > @@ -917,39 +1379,131 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > return IRQ_HANDLED;
> > }
> >
> > -static void mtk_jpeg_set_default_params(struct mtk_jpeg_ctx *ctx)
> > +static void mtk_jpeg_set_enc_default_params(struct mtk_jpeg_ctx *ctx)
> > {
> > struct mtk_jpeg_q_data *q = &ctx->out_q;
> > - int i;
> > + struct v4l2_pix_format_mplane *pix_mp;
> > +
> > + pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL);
> >
> > + ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> > ctx->colorspace = V4L2_COLORSPACE_JPEG,
> > ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> > ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > -
> > - q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_JPEG,
> > - MTK_JPEG_FMT_TYPE_OUTPUT);
> > - q->w = MTK_JPEG_MIN_WIDTH;
> > - q->h = MTK_JPEG_MIN_HEIGHT;
> > - q->bytesperline[0] = 0;
> > - q->sizeimage[0] = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > + pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > + pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +
> > + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUYV,
> > + MTK_JPEG_FMT_FLAG_ENC_OUTPUT);
> > + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > + fmt.pix_mp), q->fmt);
> > + q->w = pix_mp->width;
> > + q->h = pix_mp->height;
> > + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> >
> > q = &ctx->cap_q;
> > - q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_YUV420M,
> > - MTK_JPEG_FMT_TYPE_CAPTURE);
> > - q->w = MTK_JPEG_MIN_WIDTH;
> > - q->h = MTK_JPEG_MIN_HEIGHT;
> > + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG,
> > + MTK_JPEG_FMT_FLAG_ENC_CAPTURE);
> > + pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > + pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > + fmt.pix_mp), q->fmt);
> > + q->w = pix_mp->width;
> > + q->h = pix_mp->height;
> > + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> > +}
> > +
> > +static void mtk_jpeg_set_dec_default_params(struct mtk_jpeg_ctx *ctx)
> > +{
> > + struct mtk_jpeg_q_data *q = &ctx->out_q;
> > + struct v4l2_pix_format_mplane *pix_mp;
> > + int i;
> > +
> > + pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL);
> >
> > + ctx->fh.ctrl_handler = &ctx->ctrl_hdl;
> > + ctx->colorspace = V4L2_COLORSPACE_JPEG,
> > + ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > + ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
> > + ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> > + pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > + pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > +
> > + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG,
> > + MTK_JPEG_FMT_FLAG_DEC_OUTPUT);
> > + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > + fmt.pix_mp), q->fmt);
> > + q->w = pix_mp->width;
> > + q->h = pix_mp->height;
> > + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage;
> > + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;
> > +
> > + q = &ctx->cap_q;
> > + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUV420M,
> > + MTK_JPEG_FMT_FLAG_DEC_CAPTURE);
> > + pix_mp->width = MTK_JPEG_MIN_WIDTH;
> > + pix_mp->height = MTK_JPEG_MIN_HEIGHT;
> > + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format,
> > + fmt.pix_mp), q->fmt);
> > + q->w = pix_mp->width;
> > + q->h = pix_mp->height;
> > for (i = 0; i < q->fmt->colplanes; i++) {
> > - u32 stride = q->w * q->fmt->h_sample[i] / 4;
> > - u32 h = q->h * q->fmt->v_sample[i] / 4;
> > + q->sizeimage[i] = pix_mp->plane_fmt[i].sizeimage;
> > + q->bytesperline[i] = pix_mp->plane_fmt[i].bytesperline;
> > + }
> > +}
> > +
> > +static int mtk_jpeg_enc_open(struct file *file)
> > +{
> > + struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > + struct video_device *vfd = video_devdata(file);
> > + struct mtk_jpeg_ctx *ctx;
> > + int ret = 0;
> >
> > - q->bytesperline[i] = stride;
> > - q->sizeimage[i] = stride * h;
> > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return -ENOMEM;
> > +
> > + if (mutex_lock_interruptible(&jpeg->lock)) {
> > + ret = -ERESTARTSYS;
> > + goto free;
> > + }
> > +
> > + v4l2_fh_init(&ctx->fh, vfd);
> > + file->private_data = &ctx->fh;
> > + v4l2_fh_add(&ctx->fh);
> > +
> > + ctx->jpeg = jpeg;
> > + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > + mtk_jpeg_enc_queue_init);
> > + if (IS_ERR(ctx->fh.m2m_ctx)) {
> > + ret = PTR_ERR(ctx->fh.m2m_ctx);
> > + goto error;
> > }
> > +
> > + ret = mtk_jpeg_enc_ctrls_setup(ctx);
> > + if (ret) {
> > + v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg enc controls\n");
> > + goto error;
> > + }
> > + mtk_jpeg_set_enc_default_params(ctx);
> > +
> > + mutex_unlock(&jpeg->lock);
> > + return 0;
> > +
> > +error:
> > + v4l2_fh_del(&ctx->fh);
> > + v4l2_fh_exit(&ctx->fh);
> > + mutex_unlock(&jpeg->lock);
> > +free:
> > + kfree(ctx);
> > + return ret;
> > }
> >
> > -static int mtk_jpeg_open(struct file *file)
> > +static int mtk_jpeg_dec_open(struct file *file)
> > {
> > struct mtk_jpeg_dev *jpeg = video_drvdata(file);
> > struct video_device *vfd = video_devdata(file);
> > @@ -971,13 +1525,20 @@ static int mtk_jpeg_open(struct file *file)
> >
> > ctx->jpeg = jpeg;
> > ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> > - mtk_jpeg_queue_init);
> > + mtk_jpeg_dec_queue_init);
> > if (IS_ERR(ctx->fh.m2m_ctx)) {
> > ret = PTR_ERR(ctx->fh.m2m_ctx);
> > goto error;
> > }
> >
> > - mtk_jpeg_set_default_params(ctx);
> > + v4l2_ctrl_handler_init(&ctx->ctrl_hdl, 0);
> > + ret = v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
> > + if (ret) {
> > + v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg dec controls\n");
> > + goto error;
> > + }
> > + mtk_jpeg_set_dec_default_params(ctx);
> > +
> > mutex_unlock(&jpeg->lock);
> > return 0;
> >
> > @@ -997,6 +1558,7 @@ static int mtk_jpeg_release(struct file *file)
> >
> > mutex_lock(&jpeg->lock);
> > v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> > + v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> > v4l2_fh_del(&ctx->fh);
> > v4l2_fh_exit(&ctx->fh);
> > kfree(ctx);
> > @@ -1004,9 +1566,18 @@ static int mtk_jpeg_release(struct file *file)
> > return 0;
> > }
> >
> > -static const struct v4l2_file_operations mtk_jpeg_fops = {
> > +static const struct v4l2_file_operations mtk_jpeg_enc_fops = {
> > .owner = THIS_MODULE,
> > - .open = mtk_jpeg_open,
> > + .open = mtk_jpeg_enc_open,
> > + .release = mtk_jpeg_release,
> > + .poll = v4l2_m2m_fop_poll,
> > + .unlocked_ioctl = video_ioctl2,
> > + .mmap = v4l2_m2m_fop_mmap,
> > +};
> > +
> > +static const struct v4l2_file_operations mtk_jpeg_dec_fops = {
> > + .owner = THIS_MODULE,
> > + .open = mtk_jpeg_dec_open,
> > .release = mtk_jpeg_release,
> > .poll = v4l2_m2m_fop_poll,
> > .unlocked_ioctl = video_ioctl2,
> > @@ -1017,6 +1588,7 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> > {
> > struct device_node *node;
> > struct platform_device *pdev;
> > + int i;
> >
> > node = of_parse_phandle(jpeg->dev->of_node, "mediatek,larb", 0);
> > if (!node)
> > @@ -1030,19 +1602,24 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> >
> > jpeg->larb = &pdev->dev;
> >
> > - jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec");
> > - if (IS_ERR(jpeg->clk_jdec))
> > - return PTR_ERR(jpeg->clk_jdec);
> > + for (i = 0; i < jpeg->variant->num_clocks; i++) {
> > + jpeg->clocks[i] = devm_clk_get(jpeg->dev,
> > + jpeg->variant->clk_names[i]);
> > + if (IS_ERR(jpeg->clocks[i])) {
> > + dev_err(&pdev->dev, "failed to get clock: %s\n",
> > + jpeg->variant->clk_names[i]);
> > + return PTR_ERR(jpeg->clocks[i]);
> > + }
> > + }
> >
> > - jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi");
> > - return PTR_ERR_OR_ZERO(jpeg->clk_jdec_smi);
> > + return 0;
> > }
> >
> > static int mtk_jpeg_probe(struct platform_device *pdev)
> > {
> > struct mtk_jpeg_dev *jpeg;
> > struct resource *res;
> > - int dec_irq;
> > + int jpeg_irq;
> > int ret;
> >
> > jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
> > @@ -1052,25 +1629,30 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> > mutex_init(&jpeg->lock);
> > spin_lock_init(&jpeg->hw_lock);
> > jpeg->dev = &pdev->dev;
> > + jpeg->variant = of_device_get_match_data(jpeg->dev);
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - jpeg->dec_reg_base = devm_ioremap_resource(&pdev->dev, res);
> > - if (IS_ERR(jpeg->dec_reg_base)) {
> > - ret = PTR_ERR(jpeg->dec_reg_base);
> > + jpeg->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(jpeg->reg_base)) {
> > + ret = PTR_ERR(jpeg->reg_base);
> > return ret;
> > }
> >
> > - dec_irq = platform_get_irq(pdev, 0);
> > - if (dec_irq < 0) {
> > - dev_err(&pdev->dev, "Failed to get dec_irq %d.\n", dec_irq);
> > - return dec_irq;
> > + jpeg_irq = platform_get_irq(pdev, 0);
> > + if (jpeg_irq < 0) {
> > + dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n", jpeg_irq);
> > + return jpeg_irq;
> > }
> >
> > - ret = devm_request_irq(&pdev->dev, dec_irq, mtk_jpeg_dec_irq, 0,
> > - pdev->name, jpeg);
> > + if (jpeg->variant->is_encoder)
> > + ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_enc_irq,
> > + 0, pdev->name, jpeg);
> > + else
> > + ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_dec_irq,
> > + 0, pdev->name, jpeg);
> > if (ret) {
> > - dev_err(&pdev->dev, "Failed to request dec_irq %d (%d)\n",
> > - dec_irq, ret);
> > + dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n",
> > + jpeg_irq, ret);
> > goto err_req_irq;
> > }
> >
> > @@ -1087,40 +1669,50 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> > goto err_dev_register;
> > }
> >
> > - jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_m2m_ops);
> > + if (jpeg->variant->is_encoder)
> > + jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_enc_m2m_ops);
> > + else
> > + jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_dec_m2m_ops);
> > if (IS_ERR(jpeg->m2m_dev)) {
> > v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n");
> > ret = PTR_ERR(jpeg->m2m_dev);
> > goto err_m2m_init;
> > }
> >
> > - jpeg->dec_vdev = video_device_alloc();
> > - if (!jpeg->dec_vdev) {
> > + jpeg->vdev = video_device_alloc();
> > + if (!jpeg->vdev) {
> > ret = -ENOMEM;
> > - goto err_dec_vdev_alloc;
> > + goto err_vfd_jpeg_alloc;
> > }
> > - snprintf(jpeg->dec_vdev->name, sizeof(jpeg->dec_vdev->name),
> > - "%s-dec", MTK_JPEG_NAME);
> > - jpeg->dec_vdev->fops = &mtk_jpeg_fops;
> > - jpeg->dec_vdev->ioctl_ops = &mtk_jpeg_ioctl_ops;
> > - jpeg->dec_vdev->minor = -1;
> > - jpeg->dec_vdev->release = video_device_release;
> > - jpeg->dec_vdev->lock = &jpeg->lock;
> > - jpeg->dec_vdev->v4l2_dev = &jpeg->v4l2_dev;
> > - jpeg->dec_vdev->vfl_dir = VFL_DIR_M2M;
> > - jpeg->dec_vdev->device_caps = V4L2_CAP_STREAMING |
> > + snprintf(jpeg->vdev->name, sizeof(jpeg->vdev->name),
> > + "%s-%s", MTK_JPEG_NAME,
> > + jpeg->variant->is_encoder ? "enc" : "dec");
> > + if (jpeg->variant->is_encoder) {
> > + jpeg->vdev->fops = &mtk_jpeg_enc_fops;
> > + jpeg->vdev->ioctl_ops = &mtk_jpeg_enc_ioctl_ops;
> > + } else {
> > + jpeg->vdev->fops = &mtk_jpeg_dec_fops;
> > + jpeg->vdev->ioctl_ops = &mtk_jpeg_dec_ioctl_ops;
> > + }
> > + jpeg->vdev->minor = -1;
> > + jpeg->vdev->release = video_device_release;
> > + jpeg->vdev->lock = &jpeg->lock;
> > + jpeg->vdev->v4l2_dev = &jpeg->v4l2_dev;
> > + jpeg->vdev->vfl_dir = VFL_DIR_M2M;
> > + jpeg->vdev->device_caps = V4L2_CAP_STREAMING |
> > V4L2_CAP_VIDEO_M2M_MPLANE;
> >
> > - ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, -1);
> > + ret = video_register_device(jpeg->vdev, VFL_TYPE_GRABBER, -1);
> > if (ret) {
> > v4l2_err(&jpeg->v4l2_dev, "Failed to register video device\n");
> > - goto err_dec_vdev_register;
> > + goto err_vfd_jpeg_register;
> > }
> >
> > - video_set_drvdata(jpeg->dec_vdev, jpeg);
> > + video_set_drvdata(jpeg->vdev, jpeg);
> > v4l2_info(&jpeg->v4l2_dev,
> > - "decoder device registered as /dev/video%d (%d,%d)\n",
> > - jpeg->dec_vdev->num, VIDEO_MAJOR, jpeg->dec_vdev->minor);
> > + "jpeg %s device registered as /dev/video%d (%d,%d)\n",
> > + jpeg->variant->is_encoder ? "enc" : "dec", jpeg->vdev->num,
> > + VIDEO_MAJOR, jpeg->vdev->minor);
> >
> > platform_set_drvdata(pdev, jpeg);
> >
> > @@ -1128,10 +1720,10 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
> >
> > return 0;
> >
> > -err_dec_vdev_register:
> > - video_device_release(jpeg->dec_vdev);
> > +err_vfd_jpeg_register:
> > + video_device_release(jpeg->vdev);
> >
> > -err_dec_vdev_alloc:
> > +err_vfd_jpeg_alloc:
> > v4l2_m2m_release(jpeg->m2m_dev);
> >
> > err_m2m_init:
> > @@ -1151,8 +1743,8 @@ static int mtk_jpeg_remove(struct platform_device *pdev)
> > struct mtk_jpeg_dev *jpeg = platform_get_drvdata(pdev);
> >
> > pm_runtime_disable(&pdev->dev);
> > - video_unregister_device(jpeg->dec_vdev);
> > - video_device_release(jpeg->dec_vdev);
> > + video_unregister_device(jpeg->vdev);
> > + video_device_release(jpeg->vdev);
> > v4l2_m2m_release(jpeg->m2m_dev);
> > v4l2_device_unregister(&jpeg->v4l2_dev);
> >
> > @@ -1211,14 +1803,36 @@ static const struct dev_pm_ops mtk_jpeg_pm_ops = {
> > SET_RUNTIME_PM_OPS(mtk_jpeg_pm_suspend, mtk_jpeg_pm_resume, NULL)
> > };
> >
> > +static struct mtk_jpeg_variant mt8173_jpeg_drvdata = {
> > + .is_encoder = false,
> > + .clk_names = {"jpgdec-smi", "jpgdec"},
> > + .num_clocks = 2,
> > +};
> > +
> > +static struct mtk_jpeg_variant mt2701_jpeg_drvdata = {
> > + .is_encoder = false,
> > + .clk_names = {"jpgdec-smi", "jpgdec"},
> > + .num_clocks = 2,
> > +};
> > +
> > +static struct mtk_jpeg_variant mtk_jpeg_drvdata = {
> > + .is_encoder = true,
> > + .clk_names = {"jpgenc"},
> > + .num_clocks = 1,
> > +};
> > +
> > static const struct of_device_id mtk_jpeg_match[] = {
> > {
> > .compatible = "mediatek,mt8173-jpgdec",
> > - .data = NULL,
> > + .data = &mt8173_jpeg_drvdata,
> > },
> > {
> > .compatible = "mediatek,mt2701-jpgdec",
> > - .data = NULL,
> > + .data = &mt2701_jpeg_drvdata,
> > + },
> > + {
> > + .compatible = "mediatek,mtk-jpgenc",
> > + .data = &mtk_jpeg_drvdata,
> > },
> > {},
> > };
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 9bbd615b1067..8f80f2a69d45 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -3,6 +3,7 @@
> > * Copyright (c) 2016 MediaTek Inc.
> > * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > * Rick Chang <rick.chang@mediatek.com>
> > + * Xia Jiang <xia.jiang@mediatek.com>
> > */
> >
> > #ifndef _MTK_JPEG_CORE_H
> > @@ -16,19 +17,21 @@
> > #define MTK_JPEG_NAME "mtk-jpeg"
> >
> > #define MTK_JPEG_COMP_MAX 3
> > +#define MTK_JPEG_MAX_CLOCKS 2
> > +
> >
> > #define MTK_JPEG_FMT_FLAG_DEC_OUTPUT BIT(0)
> > #define MTK_JPEG_FMT_FLAG_DEC_CAPTURE BIT(1)
> > -
> > -#define MTK_JPEG_FMT_TYPE_OUTPUT 1
> > -#define MTK_JPEG_FMT_TYPE_CAPTURE 2
> > +#define MTK_JPEG_FMT_FLAG_ENC_OUTPUT BIT(2)
> > +#define MTK_JPEG_FMT_FLAG_ENC_CAPTURE BIT(3)
> >
> > #define MTK_JPEG_MIN_WIDTH 32U
> > #define MTK_JPEG_MIN_HEIGHT 32U
> > -#define MTK_JPEG_MAX_WIDTH 8192U
> > -#define MTK_JPEG_MAX_HEIGHT 8192U
> > +#define MTK_JPEG_MAX_WIDTH 65535U
> > +#define MTK_JPEG_MAX_HEIGHT 65535U
> >
> > #define MTK_JPEG_DEFAULT_SIZEIMAGE (1 * 1024 * 1024)
> > +#define MTK_JPEG_DEFAULT_EXIF_SIZE (64 * 1024)
> >
> > /**
> > * enum mtk_jpeg_ctx_state - contex state of jpeg
> > @@ -39,6 +42,18 @@ enum mtk_jpeg_ctx_state {
> > MTK_JPEG_SOURCE_CHANGE,
> > };
> >
> > +/**
> > + * mtk_jpeg_variant - mtk jpeg driver variant
> > + * @is_encoder: driver mode is jpeg encoder
> > + * @clk_names: clock names
> > + * @num_clocks: numbers of clock
> > + */
> > +struct mtk_jpeg_variant {
> > + bool is_encoder;
> > + const char *clk_names[MTK_JPEG_MAX_CLOCKS];
> > + int num_clocks;
> > +};
> > +
> > /**
> > * struct mt_jpeg - JPEG IP abstraction
> > * @lock: the mutex protecting this structure
> > @@ -48,11 +63,11 @@ enum mtk_jpeg_ctx_state {
> > * @v4l2_dev: v4l2 device for mem2mem mode
> > * @m2m_dev: v4l2 mem2mem device data
> > * @alloc_ctx: videobuf2 memory allocator's context
> > - * @dec_vdev: video device node for decoder mem2mem mode
> > - * @dec_reg_base: JPEG registers mapping
> > - * @clk_jdec: JPEG hw working clock
> > - * @clk_jdec_smi: JPEG SMI bus clock
> > + * @vdev: video device node for jpeg mem2mem mode
> > + * @reg_base: JPEG registers mapping
> > * @larb: SMI device
> > + * @clocks: JPEG IP clock(s)
> > + * @variant: driver variant to be used
> > */
> > struct mtk_jpeg_dev {
> > struct mutex lock;
> > @@ -62,16 +77,17 @@ struct mtk_jpeg_dev {
> > struct v4l2_device v4l2_dev;
> > struct v4l2_m2m_dev *m2m_dev;
> > void *alloc_ctx;
> > - struct video_device *dec_vdev;
> > - void __iomem *dec_reg_base;
> > - struct clk *clk_jdec;
> > - struct clk *clk_jdec_smi;
> > + struct video_device *vdev;
> > + void __iomem *reg_base;
> > struct device *larb;
> > + struct clk *clocks[MTK_JPEG_MAX_CLOCKS];
> > + const struct mtk_jpeg_variant *variant;
> > };
> >
> > /**
> > * struct jpeg_fmt - driver's internal color format data
> > * @fourcc: the fourcc code, 0 if not applicable
> > + * @hw_format: hardware format value
> > * @h_sample: horizontal sample count of plane in 4 * 4 pixel image
> > * @v_sample: vertical sample count of plane in 4 * 4 pixel image
> > * @colplanes: number of color planes (1 for packed formats)
> > @@ -81,6 +97,7 @@ struct mtk_jpeg_dev {
> > */
> > struct mtk_jpeg_fmt {
> > u32 fourcc;
> > + u32 hw_format;
> > int h_sample[VIDEO_MAX_PLANES];
> > int v_sample[VIDEO_MAX_PLANES];
> > int colplanes;
> > @@ -113,6 +130,10 @@ struct mtk_jpeg_q_data {
> > * @cap_q: destination (capture) queue queue information
> > * @fh: V4L2 file handle
> > * @state: state of the context
> > + * @enable_exif: enable exif mode of jpeg encoder
> > + * @enc_quality: jpeg encoder quality
> > + * @restart_interval: jpeg encoder restart interval
> > + * @ctrl_hdl: controls handler
> > * @colorspace: enum v4l2_colorspace; supplemental to pixelformat
> > * @ycbcr_enc: enum v4l2_ycbcr_encoding, Y'CbCr encoding
> > * @quantization: enum v4l2_quantization, colorspace quantization
> > @@ -124,6 +145,10 @@ struct mtk_jpeg_ctx {
> > struct mtk_jpeg_q_data cap_q;
> > struct v4l2_fh fh;
> > enum mtk_jpeg_ctx_state state;
> > + bool enable_exif;
> > + u8 enc_quality;
> > + u8 restart_interval;
> > + struct v4l2_ctrl_handler ctrl_hdl;
> >
> > enum v4l2_colorspace colorspace;
> > enum v4l2_ycbcr_encoding ycbcr_enc;
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > index 1cc37dbfc8e7..ce263db5f30a 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
> > @@ -3,10 +3,11 @@
> > * Copyright (c) 2016 MediaTek Inc.
> > * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com>
> > * Rick Chang <rick.chang@mediatek.com>
> > + * Xia Jiang <xia.jiang@mediatek.com>
> > */
> >
> > -#ifndef _MTK_JPEG_HW_H
> > -#define _MTK_JPEG_HW_H
> > +#ifndef _MTK_JPEG_DEC_HW_H
> > +#define _MTK_JPEG_DEC_HW_H
> >
> > #include <media/videobuf2-core.h>
> >
> > @@ -75,4 +76,4 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
> > void mtk_jpeg_dec_reset(void __iomem *dec_reg_base);
> > void mtk_jpeg_dec_start(void __iomem *dec_reg_base);
> >
> > -#endif /* _MTK_JPEG_HW_H */
> > +#endif /* _MTK_JPEG_DEC_HW_H */
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > new file mode 100644
> > index 000000000000..7fc1de920a75
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > @@ -0,0 +1,193 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2019 MediaTek Inc.
> > + * Author: Xia Jiang <xia.jiang@mediatek.com>
> > + *
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_jpeg_enc_hw.h"
> > +
> > +static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {
> > + {.quality_param = 34, .hardware_value = JPEG_ENC_QUALITY_Q34},
> > + {.quality_param = 39, .hardware_value = JPEG_ENC_QUALITY_Q39},
> > + {.quality_param = 48, .hardware_value = JPEG_ENC_QUALITY_Q48},
> > + {.quality_param = 60, .hardware_value = JPEG_ENC_QUALITY_Q60},
> > + {.quality_param = 64, .hardware_value = JPEG_ENC_QUALITY_Q64},
> > + {.quality_param = 68, .hardware_value = JPEG_ENC_QUALITY_Q68},
> > + {.quality_param = 74, .hardware_value = JPEG_ENC_QUALITY_Q74},
> > + {.quality_param = 80, .hardware_value = JPEG_ENC_QUALITY_Q80},
> > + {.quality_param = 82, .hardware_value = JPEG_ENC_QUALITY_Q82},
> > + {.quality_param = 84, .hardware_value = JPEG_ENC_QUALITY_Q84},
> > + {.quality_param = 87, .hardware_value = JPEG_ENC_QUALITY_Q87},
> > + {.quality_param = 90, .hardware_value = JPEG_ENC_QUALITY_Q90},
> > + {.quality_param = 92, .hardware_value = JPEG_ENC_QUALITY_Q92},
> > + {.quality_param = 95, .hardware_value = JPEG_ENC_QUALITY_Q95},
> > + {.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97},
> > +};
> > +
> > +void mtk_jpeg_enc_reset(void __iomem *base)
> > +{
> > + writel(0x00, base + JPEG_ENC_RSTB);
> > + writel(JPEG_ENC_RESET_BIT, base + JPEG_ENC_RSTB);
> > + writel(0x00, base + JPEG_ENC_CODEC_SEL);
> > +}
> > +
> > +u32 mtk_jpeg_enc_get_and_clear_int_status(void __iomem *base)
> > +{
> > + u32 ret;
> > +
> > + ret = readl(base + JPEG_ENC_INT_STS) &
> > + JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
> > + if (ret)
> > + writel(0, base + JPEG_ENC_INT_STS);
> > +
> > + return ret;
> > +}
> > +
> > +u32 mtk_jpeg_enc_get_file_size(void __iomem *base)
> > +{
> > + return readl(base + JPEG_ENC_DMA_ADDR0) -
> > + readl(base + JPEG_ENC_DST_ADDR0);
> > +}
> > +
> > +u32 mtk_jpeg_enc_enum_result(void __iomem *base, u32 irq_status)
> > +{
> > + if (irq_status & JPEG_ENC_INT_STATUS_DONE)
> > + return MTK_JPEG_ENC_RESULT_DONE;
> > + else if (irq_status & JPEG_ENC_INT_STATUS_STALL)
> > + return MTK_JPEG_ENC_RESULT_STALL;
> > + else
> > + return MTK_JPEG_ENC_RESULT_VCODEC_IRQ;
> > +}
> > +
> > +void mtk_jpeg_enc_set_img_size(void __iomem *base, u32 width, u32 height)
> > +{
> > + u32 value;
> > +
> > + value = width << 16 | height;
> > + writel(value, base + JPEG_ENC_IMG_SIZE);
> > +}
> > +
> > +void mtk_jpeg_enc_set_blk_num(void __iomem *base, u32 enc_format, u32 width,
> > + u32 height)
> > +{
> > + u32 blk_num;
> > + u32 is_420;
> > + u32 padding_width;
> > + u32 padding_height;
> > + u32 luma_blocks;
> > + u32 chroma_blocks;
> > +
> > + is_420 = (enc_format == V4L2_PIX_FMT_NV12M ||
> > + enc_format == V4L2_PIX_FMT_NV21M) ? 1 : 0;
> > + padding_width = round_up(width, 16);
> > + padding_height = round_up(height, is_420 ? 16 : 8);
> > +
> > + luma_blocks = padding_width / 8 * padding_height / 8;
> > + if (is_420)
> > + chroma_blocks = luma_blocks / 4;
> > + else
> > + chroma_blocks = luma_blocks / 2;
> > +
> > + blk_num = luma_blocks + 2 * chroma_blocks - 1;
> > +
> > + writel(blk_num, base + JPEG_ENC_BLK_NUM);
> > +}
> > +
> > +void mtk_jpeg_enc_set_stride(void __iomem *base, u32 enc_format, u32 width,
> > + u32 height, u32 bytesperline)
> > +{
> > + u32 img_stride;
> > + u32 mem_stride;
> > +
> > + if (enc_format == V4L2_PIX_FMT_NV12M ||
> > + enc_format == V4L2_PIX_FMT_NV21M) {
> > + img_stride = round_up(width, 16);
> > + mem_stride = bytesperline;
> > + } else {
> > + img_stride = round_up(width * 2, 32);
> > + mem_stride = img_stride;
> > + }
> > +
> > + writel(img_stride, base + JPEG_ENC_IMG_STRIDE);
> > + writel(mem_stride, base + JPEG_ENC_STRIDE);
> > +}
> > +
> > +void mtk_jpeg_enc_set_src_addr(void __iomem *base, u32 src_addr,
> > + u32 plane_index)
> > +{
> > + if (!plane_index)
> > + writel(src_addr, base + JPEG_ENC_SRC_LUMA_ADDR);
> > + else
> > + writel(src_addr, base + JPEG_ENC_SRC_CHROMA_ADDR);
> > +}
> > +
> > +void mtk_jpeg_enc_set_dst_addr(void __iomem *base, u32 dst_addr,
> > + u32 stall_size, u32 init_offset,
> > + u32 offset_mask)
> > +{
> > + writel(init_offset & ~0xf, base + JPEG_ENC_OFFSET_ADDR);
> > + writel(offset_mask & 0xf, base + JPEG_ENC_BYTE_OFFSET_MASK);
> > + writel(dst_addr & ~0xf, base + JPEG_ENC_DST_ADDR0);
> > + writel((dst_addr + stall_size) & ~0xf, base + JPEG_ENC_STALL_ADDR0);
> > +}
> > +
> > +static void mtk_jpeg_enc_set_quality(void __iomem *base, u32 quality)
> > +{
> > + u32 value;
> > + u32 i, enc_quality;
> > +
> > + enc_quality = mtk_jpeg_enc_quality[0].hardware_value;
> > + for (i = 0; i < ARRAY_SIZE(mtk_jpeg_enc_quality); i++) {
> > + if (quality <= mtk_jpeg_enc_quality[i].quality_param) {
> > + enc_quality = mtk_jpeg_enc_quality[i].hardware_value;
> > + break;
> > + }
> > + }
> > +
> > + value = readl(base + JPEG_ENC_QUALITY);
> > + value = (value & JPEG_ENC_QUALITY_MASK) | enc_quality;
> > + writel(value, base + JPEG_ENC_QUALITY);
> > +}
> > +
> > +static void mtk_jpeg_enc_set_ctrl(void __iomem *base, u32 enc_format,
> > + bool exif_en, u32 restart_interval)
> > +{
> > + u32 value;
> > +
> > + value = readl(base + JPEG_ENC_CTRL);
> > + value &= ~JPEG_ENC_CTRL_YUV_FORMAT_MASK;
> > + value |= (enc_format & 3) << 3;
> > + if (exif_en)
> > + value |= JPEG_ENC_CTRL_FILE_FORMAT_BIT;
> > + else
> > + value &= ~JPEG_ENC_CTRL_FILE_FORMAT_BIT;
> > + if (restart_interval)
> > + value |= JPEG_ENC_CTRL_RESTART_EN_BIT;
> > + else
> > + value &= ~JPEG_ENC_CTRL_RESTART_EN_BIT;
> > + writel(value, base + JPEG_ENC_CTRL);
> > +}
> > +
> > +void mtk_jpeg_enc_set_config(void __iomem *base, u32 enc_format, bool exif_en,
> > + u32 quality, u32 restart_interval)
> > +{
> > + mtk_jpeg_enc_set_quality(base, quality);
> > +
> > + mtk_jpeg_enc_set_ctrl(base, enc_format, exif_en, restart_interval);
> > +
> > + writel(restart_interval, base + JPEG_ENC_RST_MCU_NUM);
> > +}
> > +
> > +void mtk_jpeg_enc_start(void __iomem *base)
> > +{
> > + u32 value;
> > +
> > + value = readl(base + JPEG_ENC_CTRL);
> > + value |= JPEG_ENC_CTRL_INT_EN_BIT | JPEG_ENC_CTRL_ENABLE_BIT;
> > + writel(value, base + JPEG_ENC_CTRL);
> > +}
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > new file mode 100644
> > index 000000000000..73faf49b667c
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h
> > @@ -0,0 +1,123 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2019 MediaTek Inc.
> > + * Author: Xia Jiang <xia.jiang@mediatek.com>
> > + *
> > + */
> > +
> > +#ifndef _MTK_JPEG_ENC_HW_H
> > +#define _MTK_JPEG_ENC_HW_H
> > +
> > +#include <media/videobuf2-core.h>
> > +
> > +#include "mtk_jpeg_core.h"
> > +
> > +#define JPEG_ENC_INT_STATUS_DONE BIT(0)
> > +#define JPEG_ENC_INT_STATUS_STALL BIT(1)
> > +#define JPEG_ENC_INT_STATUS_VCODEC_IRQ BIT(4)
> > +#define JPEG_ENC_INT_STATUS_MASK_ALLIRQ 0x13
> > +
> > +#define JPEG_ENC_DST_ADDR_OFFSET_MASK GENMASK(3, 0)
> > +#define JPEG_ENC_QUALITY_MASK GENMASK(31, 16)
> > +
> > +#define JPEG_ENC_CTRL_YUV_FORMAT_MASK 0x18
> > +#define JPEG_ENC_CTRL_RESTART_EN_BIT BIT(10)
> > +#define JPEG_ENC_CTRL_FILE_FORMAT_BIT BIT(5)
> > +#define JPEG_ENC_CTRL_INT_EN_BIT BIT(2)
> > +#define JPEG_ENC_CTRL_ENABLE_BIT BIT(0)
> > +#define JPEG_ENC_RESET_BIT BIT(0)
> > +
> > +#define JPEG_ENC_YUV_FORMAT_YUYV 0
> > +#define JPEG_ENC_YUV_FORMAT_YVYU 1
> > +#define JPEG_ENC_YUV_FORMAT_NV12 2
> > +#define JEPG_ENC_YUV_FORMAT_NV21 3
> > +
> > +#define JPEG_ENC_QUALITY_Q60 0x0
> > +#define JPEG_ENC_QUALITY_Q80 0x1
> > +#define JPEG_ENC_QUALITY_Q90 0x2
> > +#define JPEG_ENC_QUALITY_Q95 0x3
> > +#define JPEG_ENC_QUALITY_Q39 0x4
> > +#define JPEG_ENC_QUALITY_Q68 0x5
> > +#define JPEG_ENC_QUALITY_Q84 0x6
> > +#define JPEG_ENC_QUALITY_Q92 0x7
> > +#define JPEG_ENC_QUALITY_Q48 0x8
> > +#define JPEG_ENC_QUALITY_Q74 0xa
> > +#define JPEG_ENC_QUALITY_Q87 0xb
> > +#define JPEG_ENC_QUALITY_Q34 0xc
> > +#define JPEG_ENC_QUALITY_Q64 0xe
> > +#define JPEG_ENC_QUALITY_Q82 0xf
> > +#define JPEG_ENC_QUALITY_Q97 0x10
> > +
> > +#define JPEG_ENC_RSTB 0x100
> > +#define JPEG_ENC_CTRL 0x104
> > +#define JPEG_ENC_QUALITY 0x108
> > +#define JPEG_ENC_BLK_NUM 0x10C
> > +#define JPEG_ENC_BLK_CNT 0x110
> > +#define JPEG_ENC_INT_STS 0x11c
> > +#define JPEG_ENC_DST_ADDR0 0x120
> > +#define JPEG_ENC_DMA_ADDR0 0x124
> > +#define JPEG_ENC_STALL_ADDR0 0x128
> > +#define JPEG_ENC_OFFSET_ADDR 0x138
> > +#define JPEG_ENC_RST_MCU_NUM 0x150
> > +#define JPEG_ENC_IMG_SIZE 0x154
> > +#define JPEG_ENC_DEBUG_INFO0 0x160
> > +#define JPEG_ENC_DEBUG_INFO1 0x164
> > +#define JPEG_ENC_TOTAL_CYCLE 0x168
> > +#define JPEG_ENC_BYTE_OFFSET_MASK 0x16c
> > +#define JPEG_ENC_SRC_LUMA_ADDR 0x170
> > +#define JPEG_ENC_SRC_CHROMA_ADDR 0x174
> > +#define JPEG_ENC_STRIDE 0x178
> > +#define JPEG_ENC_IMG_STRIDE 0x17c
> > +#define JPEG_ENC_DCM_CTRL 0x300
> > +#define JPEG_ENC_CODEC_SEL 0x314
> > +#define JPEG_ENC_ULTRA_THRES 0x318
> > +
> > +enum {
> > + MTK_JPEG_ENC_RESULT_DONE,
> > + MTK_JPEG_ENC_RESULT_STALL,
> > + MTK_JPEG_ENC_RESULT_VCODEC_IRQ
> > +};
> > +
> > +/**
> > + * struct mtk_jpeg_enc_qlt - JPEG encoder quality data
> > + * @quality_param: quality value
> > + * @hardware_value: hardware value of quality
> > + */
> > +struct mtk_jpeg_enc_qlt {
> > + u8 quality_param;
> > + u8 hardware_value;
> > +};
> > +
> > +/**
> > + * struct mt_jpeg_enc_bs - JPEG encoder bitstream buffer
> > + * @dma_addr: JPEG encoder destination address
> > + * @size: JPEG encoder bistream size
> > + * @dma_addr_offset: JPEG encoder offset address
> > + * @dma_addr_offsetmask: JPEG encoder destination address offset mask
> > + */
> > +struct mtk_jpeg_enc_bs {
> > + dma_addr_t dma_addr;
> > + size_t size;
> > + u32 dma_addr_offset;
> > + u32 dma_addr_offsetmask;
> > +};
> > +
> > +void mtk_jpeg_enc_reset(void __iomem *base);
> > +u32 mtk_jpeg_enc_get_and_clear_int_status(void __iomem *base);
> > +u32 mtk_jpeg_enc_get_file_size(void __iomem *base);
> > +u32 mtk_jpeg_enc_enum_result(void __iomem *base, u32 irq_status);
> > +void mtk_jpeg_enc_set_img_size(void __iomem *base, u32 width, u32 height);
> > +void mtk_jpeg_enc_set_blk_num(void __iomem *base, u32 enc_format, u32 width,
> > + u32 height);
> > +void mtk_jpeg_enc_set_stride(void __iomem *base, u32 enc_format, u32 width,
> > + u32 height, u32 bytesperline);
> > +void mtk_jpeg_enc_set_src_addr(void __iomem *base, u32 src_addr,
> > + u32 plane_index);
> > +void mtk_jpeg_enc_set_dst_addr(void __iomem *base, u32 dst_addr,
> > + u32 stall_size, u32 init_offset,
> > + u32 offset_mask);
> > +void mtk_jpeg_enc_set_config(void __iomem *base, u32 enc_format, bool exif_en,
> > + u32 quality, u32 restart_interval);
> > +void mtk_jpeg_enc_start(void __iomem *enc_reg_base);
> > +
> > +#endif /* _MTK_JPEG_ENC_HW_H */
> >
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 08/14] media: platform: Change case for improving code quality
From: Xia Jiang @ 2020-06-05 8:04 UTC (permalink / raw)
To: Hans Verkuil
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, Tomasz Figa, maoguang.meng,
Matthias Brugger, sj.huang, Rob Herring, linux-mediatek,
Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
linux-media
In-Reply-To: <4b8cc41e-5171-0d48-f588-96e4212ab22c@xs4all.nl>
On Mon, 2020-05-11 at 10:37 +0200, Hans Verkuil wrote:
> On 03/04/2020 11:40, Xia Jiang wrote:
> > Change register offset hex numberals from upercase to lowercase.
>
> Typos:
>
> numberals -> numerals
>
> upercase -> uppercase
Done.
>
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > index 94db04e9cdb6..2945da842dfa 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h
> > @@ -20,29 +20,29 @@
> > #define BIT_INQST_MASK_ALLIRQ 0x37
> >
> > #define JPGDEC_REG_RESET 0x0090
> > -#define JPGDEC_REG_BRZ_FACTOR 0x00F8
> > -#define JPGDEC_REG_DU_NUM 0x00FC
> > +#define JPGDEC_REG_BRZ_FACTOR 0x00f8
> > +#define JPGDEC_REG_DU_NUM 0x00fc
> > #define JPGDEC_REG_DEST_ADDR0_Y 0x0140
> > #define JPGDEC_REG_DEST_ADDR0_U 0x0144
> > #define JPGDEC_REG_DEST_ADDR0_V 0x0148
> > -#define JPGDEC_REG_DEST_ADDR1_Y 0x014C
> > +#define JPGDEC_REG_DEST_ADDR1_Y 0x014c
> > #define JPGDEC_REG_DEST_ADDR1_U 0x0150
> > #define JPGDEC_REG_DEST_ADDR1_V 0x0154
> > #define JPGDEC_REG_STRIDE_Y 0x0158
> > -#define JPGDEC_REG_STRIDE_UV 0x015C
> > +#define JPGDEC_REG_STRIDE_UV 0x015c
> > #define JPGDEC_REG_IMG_STRIDE_Y 0x0160
> > #define JPGDEC_REG_IMG_STRIDE_UV 0x0164
> > -#define JPGDEC_REG_WDMA_CTRL 0x016C
> > +#define JPGDEC_REG_WDMA_CTRL 0x016c
> > #define JPGDEC_REG_PAUSE_MCU_NUM 0x0170
> > -#define JPGDEC_REG_OPERATION_MODE 0x017C
> > +#define JPGDEC_REG_OPERATION_MODE 0x017c
> > #define JPGDEC_REG_FILE_ADDR 0x0200
> > -#define JPGDEC_REG_COMP_ID 0x020C
> > +#define JPGDEC_REG_COMP_ID 0x020c
> > #define JPGDEC_REG_TOTAL_MCU_NUM 0x0210
> > #define JPGDEC_REG_COMP0_DATA_UNIT_NUM 0x0224
> > -#define JPGDEC_REG_DU_CTRL 0x023C
> > +#define JPGDEC_REG_DU_CTRL 0x023c
> > #define JPGDEC_REG_TRIG 0x0240
> > #define JPGDEC_REG_FILE_BRP 0x0248
> > -#define JPGDEC_REG_FILE_TOTAL_SIZE 0x024C
> > +#define JPGDEC_REG_FILE_TOTAL_SIZE 0x024c
> > #define JPGDEC_REG_QT_ID 0x0270
> > #define JPGDEC_REG_INTERRUPT_STATUS 0x0274
> > #define JPGDEC_REG_STATUS 0x0278
> >
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] mt76: mt7663: introduce ARP filter offload
From: Johannes Berg @ 2020-06-05 7:21 UTC (permalink / raw)
To: Lorenzo Bianconi, nbd
Cc: linux-mediatek, lorenzo.bianconi, ryder.lee, linux-wireless,
sean.wang
In-Reply-To: <e91990d20a1a5f8d134fc9d9df152d9356fd15f9.1591340650.git.lorenzo@kernel.org>
On Fri, 2020-06-05 at 09:07 +0200, Lorenzo Bianconi wrote:
>
> + .arp = {
> + .tag = cpu_to_le16(UNI_OFFLOAD_OFFLOAD_ARP),
> + .len = cpu_to_le16(sizeof(struct mt7615_arpns_tlv)),
> + .ips_num = info->arp_addr_cnt,
Surely there's *some* hardware limit, after which you need to disable
the filter? Or does the firmware take the data and discard it, it it's
got too many IPs?
At the very least, only 255 fit in there, and at least theoretically you
can assign more to the netdev :)
johannes
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: Security Random Number Generator support
From: Neal Liu @ 2020-06-05 7:19 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Marc Zyngier
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Julius Werner, Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman,
Sean Wang, linux-mediatek@lists.infradead.org, lkml, wsd_upstream,
Rob Herring, Neal Liu, Linux Crypto Mailing List, Matt Mackall,
Matthias Brugger, Crystal Guo (郭晶), Ard Biesheuvel,
Linux ARM
In-Reply-To: <20200603093416.GY1551@shell.armlinux.org.uk>
On Wed, 2020-06-03 at 17:34 +0800, Russell King - ARM Linux admin wrote:
> On Wed, Jun 03, 2020 at 08:40:58AM +0100, Marc Zyngier wrote:
> > On 2020-06-03 08:29, Neal Liu wrote:
> > > On Tue, 2020-06-02 at 21:02 +0800, Marc Zyngier wrote:
> > > > On 2020-06-02 13:14, Ard Biesheuvel wrote:
> > > > > On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
> > > > >>
> > > > >> These patch series introduce a security random number generator
> > > > >> which provides a generic interface to get hardware rnd from Secure
> > > > >> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
> > > > >> Execution Environment(TEE), or even EL2 hypervisor.
> > > > >>
> > > > >> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
> > > > >> For security awareness SoCs on ARMv8 with TrustZone enabled,
> > > > >> peripherals like entropy sources is not accessible from normal world
> > > > >> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
> > > > >> This driver aims to provide a generic interface to Arm Trusted
> > > > >> Firmware or Hypervisor rng service.
> > > > >>
> > > > >>
> > > > >> changes since v1:
> > > > >> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
> > > > >> reuse
> > > > >> this driver.
> > > > >> - refine coding style and unnecessary check.
> > > > >>
> > > > >> changes since v2:
> > > > >> - remove unused comments.
> > > > >> - remove redundant variable.
> > > > >>
> > > > >> changes since v3:
> > > > >> - add dt-bindings for MediaTek rng with TrustZone enabled.
> > > > >> - revise HWRNG SMC call fid.
> > > > >>
> > > > >> changes since v4:
> > > > >> - move bindings to the arm/firmware directory.
> > > > >> - revise driver init flow to check more property.
> > > > >>
> > > > >> changes since v5:
> > > > >> - refactor to more generic security rng driver which
> > > > >> is not platform specific.
> > > > >>
> > > > >> *** BLURB HERE ***
> > > > >>
> > > > >> Neal Liu (2):
> > > > >> dt-bindings: rng: add bindings for sec-rng
> > > > >> hwrng: add sec-rng driver
> > > > >>
> > > > >
> > > > > There is no reason to model a SMC call as a driver, and represent it
> > > > > via a DT node like this.
> > > >
> > > > +1.
> > > >
> > > > > It would be much better if this SMC interface is made truly generic,
> > > > > and wired into the arch_get_random() interface, which can be used much
> > > > > earlier.
> > > >
> > > > Wasn't there a plan to standardize a SMC call to rule them all?
> > > >
> > > > M.
> > >
> > > Could you give us a hint how to make this SMC interface more generic in
> > > addition to my approach?
> > > There is no (easy) way to get platform-independent SMC function ID,
> > > which is why we encode it into device tree, and provide a generic
> > > driver. In this way, different devices can be mapped and then get
> > > different function ID internally.
> >
> > The idea is simply to have *one* single ID that caters for all
> > implementations, just like we did for PSCI at the time. This
> > requires ARM to edict a standard, which is what I was referring
> > to above.
>
> This sounds all too familiar.
>
> This kind of thing is something that ARM have seems to shy away from
> doing - it's a point I brought up many years ago when the whole
> trustzone thing first appeared with its SMC call. Those around the
> conference table were not interested - ARM seemed to prefer every
> vendor to do off and do their own thing with the SMC interface.
Does that mean it make sense to model a sec-rng driver, and get each
vendor's SMC function id by DT node?
>
> Then OMAP came along with its SMC interfaces, and so did the pain of
> not having a standardised way to configure the L2C when Linux was
> running in the non-secure world, resulting in stuff like l2c_configure
> etc, where each and every implementation has to supply a function to
> call its platform specific SMC interfaces to configure a piece of
> hardware common across many different platforms.
>
> ARM have seemed reluctant to standardise on stuff like this, so
> unless someone pushes hard for it from inside ARM, I doubt it will
> ever happen.
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* [PATCH] mt76: mt7663: introduce ARP filter offload
From: Lorenzo Bianconi @ 2020-06-05 7:07 UTC (permalink / raw)
To: nbd; +Cc: linux-mediatek, lorenzo.bianconi, ryder.lee, linux-wireless,
sean.wang
From: Sean Wang <sean.wang@mediatek.com>
Introduce ARP filter offload
Co-developed-by: Wan-Feng Jiang <Wan-Feng.Jiang@mediatek.com>
Signed-off-by: Wan-Feng Jiang <Wan-Feng.Jiang@mediatek.com>
Co-developed-by: Soul Huang <Soul.Huang@mediatek.com>
Signed-off-by: Soul Huang <Soul.Huang@mediatek.com>
Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
.../net/wireless/mediatek/mt76/mt7615/main.c | 3 +
.../net/wireless/mediatek/mt76/mt7615/mcu.c | 74 +++++++++++++++++++
.../net/wireless/mediatek/mt76/mt7615/mcu.h | 13 +++-
.../wireless/mediatek/mt76/mt7615/mt7615.h | 4 +-
4 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index beaca8127680..81fc4982a01f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -491,6 +491,9 @@ static void mt7615_bss_info_changed(struct ieee80211_hw *hw,
if (changed & BSS_CHANGED_PS)
mt7615_mcu_set_vif_ps(dev, vif);
+ if (changed & BSS_CHANGED_ARP_FILTER)
+ mt7615_mcu_update_arp_filter(hw, vif, info);
+
mutex_unlock(&dev->mt76.mutex);
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 6e869b8c5e26..0850b13b7007 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -3542,6 +3542,32 @@ mt7615_mcu_set_gtk_rekey(struct mt7615_dev *dev,
&req, sizeof(req), true);
}
+static int
+mt7615_mcu_set_arp_filter(struct mt7615_dev *dev, struct ieee80211_vif *vif,
+ bool suspend)
+{
+ struct mt7615_vif *mvif = (struct mt7615_vif *)vif->drv_priv;
+ struct {
+ struct {
+ u8 bss_idx;
+ u8 pad[3];
+ } __packed hdr;
+ struct mt7615_arpns_tlv arpns;
+ } req = {
+ .hdr = {
+ .bss_idx = mvif->idx,
+ },
+ .arpns = {
+ .tag = cpu_to_le16(UNI_OFFLOAD_OFFLOAD_ARP),
+ .len = cpu_to_le16(sizeof(struct mt7615_arpns_tlv)),
+ .mode = suspend,
+ },
+ };
+
+ return __mt76_mcu_send_msg(&dev->mt76, MCU_UNI_CMD_OFFLOAD,
+ &req, sizeof(req), true);
+}
+
void mt7615_mcu_set_suspend_iter(void *priv, u8 *mac,
struct ieee80211_vif *vif)
{
@@ -3554,6 +3580,7 @@ void mt7615_mcu_set_suspend_iter(void *priv, u8 *mac,
mt7615_mcu_set_bss_pm(phy->dev, vif, suspend);
mt7615_mcu_set_gtk_rekey(phy->dev, vif, suspend);
+ mt7615_mcu_set_arp_filter(phy->dev, vif, suspend);
mt7615_mcu_set_suspend_mode(phy->dev, vif, suspend, 1, true);
@@ -3653,6 +3680,53 @@ int mt7615_mcu_set_roc(struct mt7615_phy *phy, struct ieee80211_vif *vif,
sizeof(req), false);
}
+int mt7615_mcu_update_arp_filter(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_bss_conf *info)
+{
+ struct mt7615_vif *mvif = (struct mt7615_vif *)vif->drv_priv;
+ struct mt7615_dev *dev = mt7615_hw_dev(hw);
+ struct sk_buff *skb;
+ struct {
+ struct {
+ u8 bss_idx;
+ u8 pad[3];
+ } __packed hdr;
+ struct mt7615_arpns_tlv arp;
+ } req_hdr = {
+ .hdr = {
+ .bss_idx = mvif->idx,
+ },
+ .arp = {
+ .tag = cpu_to_le16(UNI_OFFLOAD_OFFLOAD_ARP),
+ .len = cpu_to_le16(sizeof(struct mt7615_arpns_tlv)),
+ .ips_num = info->arp_addr_cnt,
+ .mode = 2, /* update purpose */
+ .option = 1,
+ },
+ };
+ int i;
+
+ if (!mt7615_firmware_offload(dev))
+ return 0;
+
+ skb = mt76_mcu_msg_alloc(&dev->mt76, NULL,
+ sizeof(req_hdr) +
+ info->arp_addr_cnt * sizeof(__be32));
+ if (!skb)
+ return -ENOMEM;
+
+ skb_put_data(skb, &req_hdr, sizeof(req_hdr));
+ for (i = 0; i < info->arp_addr_cnt; i++) {
+ u8 *addr = (u8 *)skb_put(skb, sizeof(__be32));
+
+ memcpy(addr, &info->arp_addr_list[i], sizeof(__be32));
+ }
+
+ return __mt76_mcu_skb_send_msg(&dev->mt76, skb,
+ MCU_UNI_CMD_OFFLOAD, true);
+}
+
int mt7615_mcu_set_p2p_oppps(struct ieee80211_hw *hw,
struct ieee80211_vif *vif)
{
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
index 2314d0b23af1..64f7471a57bb 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.h
@@ -545,6 +545,15 @@ struct mt7615_roc_tlv {
u8 rsv1[8];
} __packed;
+struct mt7615_arpns_tlv {
+ __le16 tag;
+ __le16 len;
+ u8 mode;
+ u8 ips_num;
+ u8 option;
+ u8 pad[1];
+} __packed;
+
/* offload mcu commands */
enum {
MCU_CMD_START_HW_SCAN = MCU_CE_PREFIX | 0x03,
@@ -580,8 +589,8 @@ enum {
};
enum {
- UNI_OFFLOAD_OFFLOAD_ARPNS_IPV4,
- UNI_OFFLOAD_OFFLOAD_ARPNS_IPV6,
+ UNI_OFFLOAD_OFFLOAD_ARP,
+ UNI_OFFLOAD_OFFLOAD_ND,
UNI_OFFLOAD_OFFLOAD_GTK_REKEY,
UNI_OFFLOAD_OFFLOAD_BMC_RPY_DETECT,
};
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
index 3e7d51bf42a4..a9513a456521 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
@@ -585,7 +585,9 @@ void mt7615_mcu_set_suspend_iter(void *priv, u8 *mac,
int mt7615_mcu_update_gtk_rekey(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct cfg80211_gtk_rekey_data *key);
-
+int mt7615_mcu_update_arp_filter(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_bss_conf *info);
int __mt7663_load_firmware(struct mt7615_dev *dev);
/* usb */
--
2.26.2
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related
* Re: [PATCH v8 10/14] media: platform: Delete redundant code for improving code quality
From: Xia Jiang @ 2020-06-05 6:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521154958.GI209565@chromium.org>
On Thu, 2020-05-21 at 15:49 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:29PM +0800, Xia Jiang wrote:
> > Delete unused member variables annotation.
> > Delete unused variable definition.
> > Delete redundant log print, because V4L2 debug logs already print it.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 16 ++--------------
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 5 +++--
> > 2 files changed, 5 insertions(+), 16 deletions(-)
> >
>
> Thank you for the patch. Please see my comments inline.
>
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 4e64046a6854..9e59b9a51ef0 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -182,7 +182,6 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > struct mtk_jpeg_ctx *ctx, int q_type)
> > {
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > - struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > int i;
> >
> > memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > @@ -190,7 +189,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >
> > if (ctx->state != MTK_JPEG_INIT) {
> > mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > - goto end;
> > + return 0;
> > }
> >
> > pix_mp->num_planes = fmt->colplanes;
> > @@ -210,7 +209,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > if (pfmt->sizeimage == 0)
> > pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > - goto end;
> > + return 0;
> > }
> >
> > /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > @@ -224,20 +223,9 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> >
> > - memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
>
> This change is not mentioned in the description. I'd suggest moving it
> to a separate patch, because it's a functional change.
>
> > pfmt->bytesperline = stride;
> > pfmt->sizeimage = stride * h;
> > }
> > -end:
> > - v4l2_dbg(2, debug, &jpeg->v4l2_dev, "wxh:%ux%u\n",
> > - pix_mp->width, pix_mp->height);
> > - for (i = 0; i < pix_mp->num_planes; i++) {
> > - v4l2_dbg(2, debug, &jpeg->v4l2_dev,
> > - "plane[%d] bpl=%u, size=%u\n",
> > - i,
> > - pix_mp->plane_fmt[i].bytesperline,
> > - pix_mp->plane_fmt[i].sizeimage);
> > - }
> > return 0;
> > }
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 64a731261214..9bbd615b1067 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -30,6 +30,9 @@
> >
> > #define MTK_JPEG_DEFAULT_SIZEIMAGE (1 * 1024 * 1024)
> >
> > +/**
> > + * enum mtk_jpeg_ctx_state - contex state of jpeg
>
> typo: s/contex/context/
>
> But I'd rephrase it to "states of the context state machine".
>
> > + */
>
> Not mentioned in the description. Also, the documentation of an enum
> should have descriptions for the values.
Done.
>
> Best regards,
> Tomasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 07/14] media: platform: Use kernel native functions for improving code quality
From: Xia Jiang @ 2020-06-05 6:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521154137.GG209565@chromium.org>
On Thu, 2020-05-21 at 15:41 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:26PM +0800, Xia Jiang wrote:
>
> Thank you for the patch. Please see my comments inline.
>
> nit: I'd remove "for improving code quality" from the subject, as it's
> obvious that we don't intend to make the code quality worse. ;)
> On the contrary, I'd make it more specific, e.g.
>
> media: mtk-jpeg: Use generic rounding helpers
>
> WDYT?
Done
>
> > Use clamp() to replace mtk_jpeg_bound_align_image() and round() to
> > replace mtk_jpeg_align().
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 41 +++++--------------
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 8 ++--
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c | 8 ++--
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h | 5 ---
> > 4 files changed, 19 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 2fa3711fdc9b..4e64046a6854 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -157,25 +157,6 @@ static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> > return NULL;
> > }
> >
> > -static void mtk_jpeg_bound_align_image(u32 *w, unsigned int wmin,
> > - unsigned int wmax, unsigned int walign,
> > - u32 *h, unsigned int hmin,
> > - unsigned int hmax, unsigned int halign)
> > -{
> > - int width, height, w_step, h_step;
> > -
> > - width = *w;
> > - height = *h;
> > - w_step = 1 << walign;
> > - h_step = 1 << halign;
> > -
> > - v4l_bound_align_image(w, wmin, wmax, walign, h, hmin, hmax, halign, 0);
> > - if (*w < width && (*w + w_step) <= wmax)
> > - *w += w_step;
> > - if (*h < height && (*h + h_step) <= hmax)
> > - *h += h_step;
> > -}
> > -
> > static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > struct v4l2_format *f)
> > {
> > @@ -218,25 +199,25 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> > struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> >
> > - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > - MTK_JPEG_MAX_WIDTH, 0,
> > - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > - MTK_JPEG_MAX_HEIGHT, 0);
> > + pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > + MTK_JPEG_MAX_HEIGHT);
> > + pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > + MTK_JPEG_MAX_WIDTH);
> >
> > memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > pfmt->bytesperline = 0;
> > /* Source size must be aligned to 128 */
> > - pfmt->sizeimage = mtk_jpeg_align(pfmt->sizeimage, 128);
> > + pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > if (pfmt->sizeimage == 0)
> > pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > goto end;
> > }
> >
> > /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > - MTK_JPEG_MAX_WIDTH, fmt->h_align,
> > - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > - MTK_JPEG_MAX_HEIGHT, fmt->v_align);
> > + pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > + MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> > + pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > + MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> >
> > for (i = 0; i < fmt->colplanes; i++) {
> > struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > @@ -751,8 +732,8 @@ static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> > {
> > bs->str_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> > bs->end_addr = bs->str_addr +
> > - mtk_jpeg_align(vb2_get_plane_payload(src_buf, 0), 16);
> > - bs->size = mtk_jpeg_align(vb2_plane_size(src_buf, 0), 128);
> > + round_up(vb2_get_plane_payload(src_buf, 0), 16);
> > + bs->size = round_up(vb2_plane_size(src_buf, 0), 128);
> > }
> >
> > static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 999bd1427809..28e9b30ad5c3 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -21,10 +21,10 @@
> > #define MTK_JPEG_FMT_TYPE_OUTPUT 1
> > #define MTK_JPEG_FMT_TYPE_CAPTURE 2
> >
> > -#define MTK_JPEG_MIN_WIDTH 32
> > -#define MTK_JPEG_MIN_HEIGHT 32
> > -#define MTK_JPEG_MAX_WIDTH 8192
> > -#define MTK_JPEG_MAX_HEIGHT 8192
> > +#define MTK_JPEG_MIN_WIDTH 32U
> > +#define MTK_JPEG_MIN_HEIGHT 32U
> > +#define MTK_JPEG_MAX_WIDTH 8192U
> > +#define MTK_JPEG_MAX_HEIGHT 8192U
>
> This change is not mentioned in the commit message. It should go to a
> separate patch, possibly merged with other really minor stylistic changes
> like this, e.g. patch 08/14.
Done
>
> Otherwise the patch looks good, so after addressing the above minor changes
> please feel free to add
>
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
>
> Best regards,
> Tomasz
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 05/14] media: platform: Improve power on and power off flow
From: Xia Jiang @ 2020-06-05 6:03 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521152253.GE209565@chromium.org>
On Thu, 2020-05-21 at 15:22 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:24PM +0800, Xia Jiang wrote:
> > Call pm_runtime_get_sync() before starting a frame and then
> > pm_runtime_put() after completing it. This can save power for the time
> > between processing two frames.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 27 +++++--------------
> > 1 file changed, 6 insertions(+), 21 deletions(-)
> >
>
> Thank you for the patch. Please see my comments inline.
>
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index a536fa95b3d6..dd5cadd101ef 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -710,23 +710,6 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> > return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > }
> >
> > -static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> > -{
> > - struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > - struct vb2_v4l2_buffer *vb;
> > - int ret = 0;
> > -
> > - ret = pm_runtime_get_sync(ctx->jpeg->dev);
> > - if (ret < 0)
> > - goto err;
> > -
> > - return 0;
> > -err:
> > - while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > - v4l2_m2m_buf_done(vb, VB2_BUF_STATE_QUEUED);
> > - return ret;
> > -}
> > -
> > static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > {
> > struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > @@ -751,8 +734,6 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> >
> > while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > -
> > - pm_runtime_put_sync(ctx->jpeg->dev);
> > }
> >
> > static const struct vb2_ops mtk_jpeg_qops = {
> > @@ -761,7 +742,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
> > .buf_queue = mtk_jpeg_buf_queue,
> > .wait_prepare = vb2_ops_wait_prepare,
> > .wait_finish = vb2_ops_wait_finish,
> > - .start_streaming = mtk_jpeg_start_streaming,
> > .stop_streaming = mtk_jpeg_stop_streaming,
> > };
> >
> > @@ -812,7 +792,7 @@ static void mtk_jpeg_device_run(void *priv)
> > struct mtk_jpeg_src_buf *jpeg_src_buf;
> > struct mtk_jpeg_bs bs;
> > struct mtk_jpeg_fb fb;
> > - int i;
> > + int i, ret;
> >
> > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > @@ -832,6 +812,10 @@ static void mtk_jpeg_device_run(void *priv)
> > return;
> > }
> >
> > + ret = pm_runtime_get_sync(jpeg->dev);
> > + if (ret < 0)
> > + goto dec_end;
> > +
> > mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> > if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> > goto dec_end;
> > @@ -957,6 +941,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > v4l2_m2m_buf_done(src_buf, buf_state);
> > v4l2_m2m_buf_done(dst_buf, buf_state);
> > v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + pm_runtime_put_sync(ctx->jpeg->dev);
>
> The _sync variant explicitly waits until the asynchronous PM operation
> completes. This is usually undesired, because the CPU stays blocked for
> no good reason. In this context it is actually a bug, because this is an
> interrupt handler and it's not allowed to sleep. I wonder why this
> actually didn't crash in your testing. Please change to the regular
> pm_runtime_put().
Done.
>
> Best regards,
> Tomasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 04/14] media: platform: Change the fixed device node number to unfixed value
From: Xia Jiang @ 2020-06-05 6:02 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521135937.GD209565@chromium.org>
On Thu, 2020-05-21 at 13:59 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:23PM +0800, Xia Jiang wrote:
> > Change device node number from 3 to -1 because that the driver will
> > also support jpeg encoder.
> >
>
> Thanks for the patch. The change is correct, but I think the commit
> message doesn't really explain the real reason for it. Perhaps something
> like
>
> "The driver can be instantiated multiple times, e.g. for a decoder and
> an encoder. Moreover, other drivers could coexist on the same system.
> This makes the static video node number assignment pointless, so switch
> to automatic assignment instead."
>
> WDYT?
Dear Tomasz,
Thanks for your advice.I have changed it in new v9 .
Best Regards,
Xia Jiang
>
> Best regards,
> Tomasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
From: Michael Kao @ 2020-06-05 3:50 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Mark Rutland, devicetree, srv_heupstream, linux-pm, linux-kernel,
Eduardo Valentin, Rob Herring, linux-mediatek, hsinyi,
Matthias Brugger, Zhang Rui, linux-arm-kernel
In-Reply-To: <1afbf412-fbeb-8abe-66d8-bd7ac4e9dd83@linaro.org>
On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote:
> On 23/03/2020 13:15, Michael Kao wrote:
> > From: "michael.kao" <michael.kao@mediatek.com>
> >
> > The driver of thermal and svs will use the
> > same register for the project which should select
> > bank before reading sensor value.
>
> Here there is a design problem AFAICT. The sensor should not be using
> external locks.
>
The PTPCORESEL is a common register used by svs and thermal.
The thermal need to ensure PTPCORESEL register will not be changed by
svs when thermal switch bank to read raw data of temperature.
So we use svs_lock to make sure there is no conflict between the two
drivers.
>
> > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > ---
> > drivers/thermal/mtk_thermal.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index 9eaca432920e..594ad4f0f8cd 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -22,6 +22,7 @@
> > #include <linux/thermal.h>
> > #include <linux/reset.h>
> > #include <linux/types.h>
> > +#include <linux/power/mtk_svs.h>
> >
> > /* AUXADC Registers */
> > #define AUXADC_CON1_SET_V 0x008
> > @@ -262,7 +263,7 @@ struct mtk_thermal {
> > struct clk *clk_peri_therm;
> > struct clk *clk_auxadc;
> > /* lock: for getting and putting banks */
> > - struct mutex lock;
> > + unsigned long flags;
> >
> > /* Calibration values */
> > s32 adc_ge;
> > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
> > u32 val;
> >
> > if (mt->conf->need_switch_bank) {
> > - mutex_lock(&mt->lock);
> > + mt->flags = claim_mtk_svs_lock();
> >
> > val = readl(mt->thermal_base + PTPCORESEL);
> > val &= ~0xf;
> > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> > struct mtk_thermal *mt = bank->mt;
> >
> > if (mt->conf->need_switch_bank)
> > - mutex_unlock(&mt->lock);
> > + release_mtk_svs_lock(mt->flags);
> > }
> >
> > /**
> > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > - mutex_init(&mt->lock);
> > -
> > mt->dev = &pdev->dev;
> >
> > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> >
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Dongchun Zhu @ 2020-06-05 3:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, linus.walleij, shengnan.wang, tfiga, bgolaszewski,
sj.huang, robh+dt, linux-mediatek, louis.kuo, matthias.bgg,
bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>
Hi Sakari,
On Thu, 2020-06-04 at 12:26 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> >
> > Could anyone help to review this patch?
> >
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 13 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 1040 insertions(+)
> > > create mode 100644 drivers/media/i2c/ov02a10.c
> > >
> >
> > [snip]
> >
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct ov02a10 *ov02a10;
> > > + unsigned int rotation;
> > > + unsigned int clock_lane_tx_speed;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > + if (!ov02a10)
> > > + return -ENOMEM;
> > > +
> > > + ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to check HW configuration: %d", ret);
> > > + return ret;
> > > + }
> > > +
> > > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > + ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > + /* Optional indication of physical rotation of sensor */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > + if (!ret && rotation == 180) {
> > > + ov02a10->upside_down = true;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > + }
> > > +
> > > + /* Optional indication of mipi TX speed */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > + &clock_lane_tx_speed);
> > > +
> > > + if (!ret)
> > > + ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > + /* Get system clock (eclk) */
> > > + ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > + if (IS_ERR(ov02a10->eclk)) {
> > > + ret = PTR_ERR(ov02a10->eclk);
> > > + dev_err(dev, "failed to get eclk %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > + &ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get eclk frequency\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(ov02a10->pd_gpio)) {
> > > + ret = PTR_ERR(ov02a10->pd_gpio);
> > > + dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > + ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > + dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > + ov02a10->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + mutex_init(&ov02a10->mutex);
> > > + ov02a10->cur_mode = &supported_modes[0];
> > > + ret = ov02a10_initialize_controls(ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to initialize controls\n");
> > > + goto err_destroy_mutex;
> > > + }
> > > +
> > > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to init entity pads: %d", ret);
> > > + goto err_free_handler;
> > > + }
> > > +
> > > + pm_runtime_enable(dev);
> > > + if (!pm_runtime_enabled(dev)) {
> > > + ret = ov02a10_power_on(dev);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > + goto err_free_handler;
This is actually wrong, which should be replaced of "err_clean_entity".
> > > + }
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > + if (ret) {
> > > + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > + if (!pm_runtime_enabled(dev))
> > > + ov02a10_power_off(dev);
>
> This should be moved to error handling section below.
>
Fine.
It would be abstracted as "err_async_register" in next release.
Something like:
err_async_register:
if (!pm_runtime_enabled(dev))
ov02a10_power_off(dev);
err_clean_entity:
media_entity_cleanup(&ov02a10->subdev.entity);
...
> > > + goto err_clean_entity;
> > > + }
> >
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> >
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> > ov02a10_power_off(dev);
> > if (ret) {
> > dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > goto err_clean_entity;
> > }
> >
> > What's your opinions about the change?
>
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.
>
> >
> > > +
> > > + return 0;
> > > +
> > > +err_clean_entity:
> > > + media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > + v4l2_async_unregister_subdev(sd);
> > > + media_entity_cleanup(&sd->entity);
> > > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + ov02a10_power_off(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > + { .compatible = "ovti,ov02a10" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > + .driver = {
> > > + .name = "ov02a10",
> > > + .pm = &ov02a10_pm_ops,
> > > + .of_match_table = ov02a10_of_match,
> > > + },
> > > + .probe_new = &ov02a10_probe,
> > > + .remove = &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 3:28 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
linux-devicetree, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, dongchun.zhu, Louis Kuo,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <20200604081032.GG16711@paasikivi.fi.intel.com>
Hi Sakari,
On Thu, 2020-06-04 at 11:10 +0300, Sakari Ailus wrote:
> On Thu, Jun 04, 2020 at 10:33:38AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz,
> >
> > On Mon, 2020-06-01 at 20:47 +0200, Tomasz Figa wrote:
> > > On Wed, May 27, 2020 at 11:03 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Mon, 2020-05-25 at 13:45 +0200, Tomasz Figa wrote:
> > > > > On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > Thanks for the review. My replies are as below.
> > > > > >
> > > > > > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > > > > > Hi Dongchun, Sakari,
> > > > > > >
> > > > > > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> > > > > [snip]
> > > > > > > > + pm_runtime_enable(dev);
> > > > > > > > + if (!pm_runtime_enabled(dev)) {
> > > > > > > > + ret = dw9768_runtime_resume(dev);
> > > > > > > > + if (ret < 0) {
> > > > > > > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > > > > > > + goto entity_cleanup;
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > > > > + if (ret < 0)
> > > > > > > > + goto entity_cleanup;
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +
> > > > > > > > +entity_cleanup:
> > > > > > >
> > > > > > > Need to power off if the code above powered on.
> > > > > > >
> > > > > >
> > > > > > Thanks for the reminder.
> > > > > > If there is something wrong with runtime PM, actuator is to be powered
> > > > > > on via dw9768_runtime_resume() API.
> > > > > > When actuator sub-device is powered on completely and async registered
> > > > > > successfully, we shall power off it afterwards.
> > > > > >
> > > > >
> > > > > The code above calls dw9768_runtime_resume() if
> > > > > !pm_runtime_enabled(dev), but the clean-up code below the
> > > > > entity_cleanup label doesn't have the corresponding
> > > > > dw9768_runtime_suspend() call.
> > > > >
> > > >
> > > > Did you mean the 'entity_cleanup' after v4l2_async_register_subdev()?
> > >
> > > Yes.
> > >
> > > > Actually I made some changes for OV02A V9, according to this comment.
> > > > Could you help review that change? Thanks.
> > >
> > > Sure, I will take a look.
> > >
> >
> > Thanks.
> > Sorry, I just wanna make sure the change is okay for next release.
> > May we use the check like OV02A V9 did?
> > ret = v4l2_async_register_subdev(&dw9768->sd);
> > if (ret < 0) {
> > if (!pm_runtime_enabled(dev))
> > dw9768_runtime_suspend(dev);
>
> Please do this part where you're jumping to, if possible.
>
Fine.
Fixed in next release.
> > goto entity_cleanup;
> > }
> >
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Tomasz Figa @ 2020-06-04 18:05 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
linux-devicetree, Linus Walleij,
Shengnan Wang (王圣男), Bartosz Golaszewski,
Sj Huang, Rob Herring, moderated list:ARM/Mediatek SoC support,
Dongchun Zhu, Louis Kuo, Matthias Brugger, Cao Bing Bu,
Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>
On Thu, Jun 4, 2020 at 11:26 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Dongchun,
>
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> >
> > Could anyone help to review this patch?
> >
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 13 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 1040 insertions(+)
> > > create mode 100644 drivers/media/i2c/ov02a10.c
> > >
> >
> > [snip]
> >
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct ov02a10 *ov02a10;
> > > + unsigned int rotation;
> > > + unsigned int clock_lane_tx_speed;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > + if (!ov02a10)
> > > + return -ENOMEM;
> > > +
> > > + ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to check HW configuration: %d", ret);
> > > + return ret;
> > > + }
> > > +
> > > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > + ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > + /* Optional indication of physical rotation of sensor */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > + if (!ret && rotation == 180) {
> > > + ov02a10->upside_down = true;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > + }
> > > +
> > > + /* Optional indication of mipi TX speed */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > + &clock_lane_tx_speed);
> > > +
> > > + if (!ret)
> > > + ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > + /* Get system clock (eclk) */
> > > + ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > + if (IS_ERR(ov02a10->eclk)) {
> > > + ret = PTR_ERR(ov02a10->eclk);
> > > + dev_err(dev, "failed to get eclk %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > + &ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get eclk frequency\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(ov02a10->pd_gpio)) {
> > > + ret = PTR_ERR(ov02a10->pd_gpio);
> > > + dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > + ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > + dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > + ov02a10->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + mutex_init(&ov02a10->mutex);
> > > + ov02a10->cur_mode = &supported_modes[0];
> > > + ret = ov02a10_initialize_controls(ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to initialize controls\n");
> > > + goto err_destroy_mutex;
> > > + }
> > > +
> > > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to init entity pads: %d", ret);
> > > + goto err_free_handler;
> > > + }
> > > +
> > > + pm_runtime_enable(dev);
> > > + if (!pm_runtime_enabled(dev)) {
> > > + ret = ov02a10_power_on(dev);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > + goto err_free_handler;
> > > + }
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > + if (ret) {
> > > + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > + if (!pm_runtime_enabled(dev))
> > > + ov02a10_power_off(dev);
>
> This should be moved to error handling section below.
>
> > > + goto err_clean_entity;
> > > + }
> >
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> >
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> > ov02a10_power_off(dev);
> > if (ret) {
> > dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > goto err_clean_entity;
> > }
> >
> > What's your opinions about the change?
>
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.
That and in general I believe the expectations are:
- runtime PM enabled - powered on only when it has something to do
- runtime PM disabled - powered on when the driver is bound (probe
succeeded), powered off when the driver unbinds (remove or probe
error)
Best regards,
Tomasz
>
> >
> > > +
> > > + return 0;
> > > +
> > > +err_clean_entity:
> > > + media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > + v4l2_async_unregister_subdev(sd);
> > > + media_entity_cleanup(&sd->entity);
> > > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + ov02a10_power_off(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > + { .compatible = "ovti,ov02a10" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > + .driver = {
> > > + .name = "ov02a10",
> > > + .pm = &ov02a10_pm_ops,
> > > + .of_match_table = ov02a10_of_match,
> > > + },
> > > + .probe_new = &ov02a10_probe,
> > > + .remove = &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
>
> --
> Sakari Ailus
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] iio: adc: mt6360: Add ADC driver for MT6360
From: Jonathan Cameron @ 2020-06-04 17:33 UTC (permalink / raw)
To: Gene Chen
Cc: gene_chen, lars, linux-iio, linux-kernel, cy_huang, benjamin.chao,
linux-mediatek, linux-arm-kernel, pmeerw, knaack.h, matthias.bgg,
Wilma.Wu, jic23, shufan_lee
In-Reply-To: <1591239631-12265-1-git-send-email-gene.chen.richtek@gmail.com>
On Thu, 4 Jun 2020 11:00:31 +0800
Gene Chen <gene.chen.richtek@gmail.com> wrote:
> From: Gene Chen <gene_chen@richtek.com>
>
> Add MT6360 ADC driver include Charger Current, Voltage, and
> Temperature.
>
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47
Hi Gene,
Comments inline.
I'd like to understand more in particularly on why the thread, rather than
using one of the standard triggers that handles that for you?
(I can think of a few reasons but better to hear from you!)
Thanks,
Jonathan
> ---
> drivers/iio/adc/Kconfig | 11 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/mt6360-adc.c | 419 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 431 insertions(+)
> create mode 100644 drivers/iio/adc/mt6360-adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 12bb8b7..a9736ec 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -657,6 +657,17 @@ config MCP3911
> This driver can also be built as a module. If so, the module will be
> called mcp3911.
>
> +config MEDIATEK_MT6360_ADC
> + tristate "Mediatek MT6360 ADC Part"
> + depends on MFD_MT6360
> + select IIO_BUFFER
> + select IIO_KFIFO_BUF
> + help
> + Say Y here to enable MT6360 ADC Part.
> + Integrated for System Monitoring include
> + Charger and Battery Current, Voltage and
> + Terperature
Temperature
> +
> config MEDIATEK_MT6577_AUXADC
> tristate "MediaTek AUXADC driver"
> depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 6378078..4209776 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_MAX9611) += max9611.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> obj-$(CONFIG_MCP3911) += mcp3911.o
> +obj-$(CONFIG_MEDIATEK_MT6360_ADC) += mt6360-adc.o
> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
> obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
> diff --git a/drivers/iio/adc/mt6360-adc.c b/drivers/iio/adc/mt6360-adc.c
> new file mode 100644
> index 0000000..bc9c488
> --- /dev/null
> +++ b/drivers/iio/adc/mt6360-adc.c
> @@ -0,0 +1,419 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + *
> + * Author: Gene Chen <gene_chen@richtek.com>
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/kthread.h>
> +#include <linux/ktime.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/mt6360.h>
> +
> +/* CHG_CTRL3 0x13 */
> +#define MT6360_AICR_MASK (0xFC)
> +#define MT6360_AICR_SHFT (2)
> +#define MT6360_AICR_400MA (0x6)
> +/* ADC_CONFIG 0x56 */
> +#define MT6360_ADCEN_SHFT (7)
> +/* ADC_RPT_1 0x5A */
> +#define MT6360_PREFERCH_MASK (0xF0)
> +#define MT6360_PREFERCH_SHFT (4)
> +#define MT6360_RPTCH_MASK (0x0F)
No need for brackets around raw numbers.
> +
> +enum {
> + MT6360_CHAN_USBID = 0,
> + MT6360_CHAN_VBUSDIV5,
> + MT6360_CHAN_VBUSDIV2,
> + MT6360_CHAN_VSYS,
> + MT6360_CHAN_VBAT,
> + MT6360_CHAN_IBUS,
> + MT6360_CHAN_IBAT,
> + MT6360_CHAN_CHG_VDDP,
> + MT6360_CHAN_TEMP_JC,
> + MT6360_CHAN_VREF_TS,
> + MT6360_CHAN_TS,
> + MT6360_CHAN_MAX,
> +};
> +
> +struct mt6360_adc_data {
> + struct device *dev;
> + struct regmap *regmap;
> + struct task_struct *scan_task;
> + struct completion adc_complete;
> + struct mutex adc_lock;
> + ktime_t last_off_timestamps[MT6360_CHAN_MAX];
> + int irq;
> +};
> +
> +static inline int mt6360_adc_val_converte(int val, int multiplier,
> + int offset, int divisor)
> +{
> + return ((val * multiplier) + offset) / divisor;
> +}
> +
> +static int mt6360_adc_get_process_val(struct mt6360_adc_data *info,
> + int chan_idx, int *val)
> +{
> + unsigned int regval = 0;
> + int ret;
> + const struct converter {
> + int multiplier;
> + int offset;
> + int divisor;
> + } adc_converter[MT6360_CHAN_MAX] = {
> + { 1250, 0, 1}, /* USBID */
> + { 6250, 0, 1}, /* VBUSDIV5 */
> + { 2500, 0, 1}, /* VBUSDIV2 */
> + { 1250, 0, 1}, /* VSYS */
> + { 1250, 0, 1}, /* VBAT */
> + { 2500, 0, 1}, /* IBUS */
> + { 2500, 0, 1}, /* IBAT */
> + { 1250, 0, 1}, /* CHG_VDDP */
> + { 105, -8000, 100}, /* TEMP_JC */
> + { 1250, 0, 1}, /* VREF_TS */
> + { 1250, 0, 1}, /* TS */
> + }, sp_ibus_adc_converter = { 1900, 0, 1 }, *sel_converter;
> +
> + if (chan_idx < 0 || chan_idx >= MT6360_CHAN_MAX)
> + return -EINVAL;
> + sel_converter = adc_converter + chan_idx;
> + if (chan_idx == MT6360_CHAN_IBUS) {
> + /* ibus chan will be affected by aicr config */
> + /* if aicr < 400, apply the special ibus converter */
> + ret = regmap_read(info->regmap, MT6360_PMU_CHG_CTRL3, ®val);
> + if (ret < 0)
> + return ret;
> + regval = (regval & MT6360_AICR_MASK) >> MT6360_AICR_SHFT;
> + if (regval < MT6360_AICR_400MA)
> + sel_converter = &sp_ibus_adc_converter;
> + }
> + *val = mt6360_adc_val_converte(*val, sel_converter->multiplier,
> + sel_converter->offset, sel_converter->divisor);
As mentioned below. Preference is always for linear conversion to be left
to consumers, either in userspace or when in kernel let the core code
deal with applying them.
So unless I'm missing something we should have offset and scale provided
via appropriate IIO_INFO elements and read_raw callbacks.
> + return 0;
> +}
> +
> +static int mt6360_adc_read_raw(struct iio_dev *iio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct mt6360_adc_data *mad = iio_priv(iio_dev);
> + long timeout;
> + u8 tmp[2], rpt[3];
> + ktime_t start_t, predict_end_t;
> + int ret;
> +
> + mutex_lock(&mad->adc_lock);
> + /* select preferred channel that we want */
> + ret = regmap_update_bits(mad->regmap,
> + MT6360_PMU_ADC_RPT_1, MT6360_PREFERCH_MASK,
> + chan->channel << MT6360_PREFERCH_SHFT);
> + if (ret < 0)
> + goto err_adc_init;
Blank line here would help readability a tiny bit.
Same in other cases where you have a statement then an error handling block.
> + /* enable adc channel we want and adc_en */
> + memset(tmp, 0, sizeof(tmp));
> + tmp[0] |= BIT(MT6360_ADCEN_SHFT);
> + tmp[(chan->channel / 8) ? 0 : 1] |= BIT(chan->channel % 8);
Hmm. This a write into a 16 bit big endian buffer I think. Would it better
to just treat it as an __be16?
> + ret = regmap_bulk_write(mad->regmap,
> + MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
> + if (ret < 0)
> + goto err_adc_init;
> + start_t = ktime_get();
> + predict_end_t = ktime_add_ms(
> + mad->last_off_timestamps[chan->channel], 50);
> + if (ktime_after(start_t, predict_end_t))
> + predict_end_t = ktime_add_ms(start_t, 25);
> + else
> + predict_end_t = ktime_add_ms(start_t, 75);
> + enable_irq(mad->irq);
So why do we need to only enable the irq here. I would have assumed it
would not happen until we trigger a read?
> +retry:
> + reinit_completion(&mad->adc_complete);
Always reinit completion before enabling the irq. Too many races happen when doing
it the other way around.
> + /* wait for conversion to complete */
> + timeout = wait_for_completion_interruptible_timeout(
> + &mad->adc_complete, msecs_to_jiffies(200));
> + if (timeout == 0) {
> + ret = -ETIMEDOUT;
> + goto err_adc_conv;
> + } else if (timeout < 0) {
> + ret = -EINTR;
> + goto err_adc_conv;
> + }
> + memset(rpt, 0, sizeof(rpt));
If reading the whole size of rpt we should never need to zero it.
> + ret = regmap_bulk_read(mad->regmap,
> + MT6360_PMU_ADC_RPT_1, rpt, sizeof(rpt));
> + if (ret < 0)
> + goto err_adc_conv;
> + /* get report channel */
> + if ((rpt[0] & MT6360_RPTCH_MASK) != chan->channel) {
> + dev_dbg(&iio_dev->dev,
> + "not wanted channel report [%02x]\n", rpt[0]);
> + goto retry;
> + }
> + if (!ktime_after(ktime_get(), predict_end_t)) {
> + dev_dbg(&iio_dev->dev, "time is not after 26ms chan_time\n");
> + goto retry;
> + }
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *val = (rpt[1] << 8) | rpt[2];
As mentioned below. It's normally an either / or for processed and raw.
When conversion is linear we prefer to push the maths to userspace.
If it's not you have no real choice but to do it in kernel and the raw
reading isn't much use.
> + break;
> + case IIO_CHAN_INFO_PROCESSED:
> + *val = (rpt[1] << 8) | rpt[2];
> + ret = mt6360_adc_get_process_val(mad, chan->channel, val);
> + if (ret < 0)
> + goto err_adc_conv;
> + break;
> + default:
> + break;
> + }
> + ret = IIO_VAL_INT;
> +err_adc_conv:
> + disable_irq(mad->irq);
> + /* whatever disable all channel and keep adc_en*/
> + memset(tmp, 0, sizeof(tmp));
> + tmp[0] |= BIT(MT6360_ADCEN_SHFT);
> + regmap_bulk_write(mad->regmap, MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
> + mad->last_off_timestamps[chan->channel] = ktime_get();
> + /* set prefer channel to 0xf */
> + regmap_update_bits(mad->regmap, MT6360_PMU_ADC_RPT_1,
> + MT6360_PREFERCH_MASK, 0xF << MT6360_PREFERCH_SHFT);
> +err_adc_init:
> + mutex_unlock(&mad->adc_lock);
> + return ret;
> +}
> +
> +static const struct iio_info mt6360_adc_iio_info = {
> + .read_raw = mt6360_adc_read_raw,
> +};
> +
> +#define MT6360_ADC_CHAN(_idx, _type) { \
> + .type = _type, \
> + .channel = MT6360_CHAN_##_idx, \
> + .scan_index = MT6360_CHAN_##_idx, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 32, \
> + .storagebits = 32, \
> + .shift = 0, \
> + .endianness = IIO_CPU, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_PROCESSED), \
It very rarely makes sense to provide both raw and processed.
Why are you doing so here?
> + .datasheet_name = #_idx, \
> + .indexed = 1, \
> +}
> +
> +static const struct iio_chan_spec mt6360_adc_channels[] = {
> + MT6360_ADC_CHAN(USBID, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(VBUSDIV5, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(VBUSDIV2, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(VSYS, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(VBAT, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(IBUS, IIO_CURRENT),
> + MT6360_ADC_CHAN(IBAT, IIO_CURRENT),
> + MT6360_ADC_CHAN(CHG_VDDP, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(TEMP_JC, IIO_TEMP),
> + MT6360_ADC_CHAN(VREF_TS, IIO_VOLTAGE),
> + MT6360_ADC_CHAN(TS, IIO_VOLTAGE),
> + IIO_CHAN_SOFT_TIMESTAMP(MT6360_CHAN_MAX),
> +};
> +
> +static irqreturn_t mt6360_pmu_adc_donei_handler(int irq, void *data)
> +{
> + struct mt6360_adc_data *mad = iio_priv(data);
> +
> + complete(&mad->adc_complete);
> + return IRQ_HANDLED;
> +}
> +
> +static int mt6360_adc_scan_task_threadfn(void *data)
> +{
> + struct mt6360_adc_data *mad = data;
> + struct iio_dev *indio_dev = iio_priv_to_dev(mad);
> + int channel_vals[MT6360_CHAN_MAX];
__aligned(8) for the above as iio_push_to_buffers_with_timestamp
needs to be able to write an aligned 8 byte timestamp.
Also is that buffer long enough to allow for that timestamp?
}
> + int i, bit, var = 0;
> + int ret;
> +
> + while (!kthread_should_stop()) {
So this is spinning as fast as possible? Seems like some sort
of delay should be in here. Why not use an existing trigger
to do this given it's sample on demand?
We have both the hrtimer trigger and a tight loop trigger
to handle usecases like this?
Is it todo with in kernel consumers?
> + memset(channel_vals, 0, sizeof(channel_vals));
> + i = 0;
> + for_each_set_bit(bit, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + ret = mt6360_adc_read_raw(indio_dev,
> + mt6360_adc_channels + bit,
> + &var, NULL,
> + IIO_CHAN_INFO_PROCESSED);
> + if (ret < 0)
> + dev_err(mad->dev, "get adc[%d] fail\n", bit);
> + channel_vals[i++] = var;
> + if (kthread_should_stop())
> + goto out;
> + }
> + iio_push_to_buffers_with_timestamp(indio_dev, channel_vals,
> + iio_get_time_ns(indio_dev));
> + }
> +out:
> + do_exit(0);
> + return 0;
> +}
> +
> +static int mt6360_adc_iio_post_enable(struct iio_dev *iio_dev)
> +{
> + struct mt6360_adc_data *mad = iio_priv(iio_dev);
> +
> + mad->scan_task = kthread_run(mt6360_adc_scan_task_threadfn, mad,
> + "scan_thread.%s", dev_name(&iio_dev->dev));
> + return PTR_ERR_OR_ZERO(mad->scan_task);
> +}
> +
> +static int mt6360_adc_iio_pre_disable(struct iio_dev *iio_dev)
> +{
> + struct mt6360_adc_data *mad = iio_priv(iio_dev);
> +
> + if (mad->scan_task) {
How could you get here without this being true?
> + kthread_stop(mad->scan_task);
> + mad->scan_task = NULL;
> + }
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops mt6360_adc_iio_setup_ops = {
> + .postenable = mt6360_adc_iio_post_enable,
> + .predisable = mt6360_adc_iio_pre_disable,
> +};
> +
> +static int mt6360_adc_iio_device_register(struct iio_dev *indio_dev)
> +{
> + struct mt6360_adc_data *mad = iio_priv(indio_dev);
> + struct iio_buffer *buffer;
> + int ret;
> +
> + indio_dev->name = dev_name(mad->dev);
> + indio_dev->dev.parent = mad->dev;
> + indio_dev->dev.of_node = mad->dev->of_node;
> + indio_dev->info = &mt6360_adc_iio_info;
> + indio_dev->channels = mt6360_adc_channels;
> + indio_dev->num_channels = ARRAY_SIZE(mt6360_adc_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> + indio_dev->setup_ops = &mt6360_adc_iio_setup_ops;
> + buffer = devm_iio_kfifo_allocate(mad->dev);
> + if (!buffer)
> + return -ENOMEM;
> + iio_device_attach_buffer(indio_dev, buffer);
> + ret = devm_iio_device_register(mad->dev, indio_dev);
> + if (ret < 0) {
Where possible use simple if (ret) as is can slightly simplify flow.
> + dev_err(mad->dev, "Failed to register iio device\n");
> + return ret;
> + }
> + return 0;
return ret; drop it out of the above brackets having changed
the check.
> +}
> +
> +static inline int mt6360_adc_reset(struct mt6360_adc_data *info)
> +{
> + u8 tmp[3] = {0x80, 0, 0};
> + ktime_t all_off_time;
> + int i;
> +
> + all_off_time = ktime_get();
> + for (i = 0; i < MT6360_CHAN_MAX; i++)
> + info->last_off_timestamps[i] = all_off_time;
> + /* enable adc_en, clear adc_chn_en/zcv/en/adc_wait_t/adc_idle_t */
> + return regmap_bulk_write(info->regmap,
> + MT6360_PMU_ADC_CONFIG, tmp, sizeof(tmp));
> +}
> +
> +static int mt6360_adc_probe(struct platform_device *pdev)
> +{
> + struct mt6360_adc_data *mad;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*mad));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + mad = iio_priv(indio_dev);
> + mad->dev = &pdev->dev;
> + init_completion(&mad->adc_complete);
> + mutex_init(&mad->adc_lock);
> + platform_set_drvdata(pdev, indio_dev);
> +
> + mad->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!mad->regmap) {
> + dev_err(&pdev->dev, "Failed to get parent regmap\n");
> + return -ENODEV;
> + }
> +
> + ret = mt6360_adc_reset(mad);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to reset adc\n");
> + return ret;
> + }
> +
> + ret = mt6360_adc_iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register iio device\n");
> + return ret;
> + }
> +
> + mad->irq = platform_get_irq_byname(pdev, "adc_donei");
> + if (mad->irq < 0) {
> + dev_err(&pdev->dev, "Failed to get adc_done irq\n");
> + return mad->irq;
> + }
> +
> + irq_set_status_flags(mad->irq, IRQ_NOAUTOEN);
> + ret = devm_request_threaded_irq(&pdev->dev, mad->irq, NULL,
> + mt6360_pmu_adc_donei_handler,
> + IRQF_TRIGGER_FALLING, "adc_donei",
> + platform_get_drvdata(pdev));
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register adc_done irq\n");
> + return ret;
> + }
It's unusual to register an interrupt 'after' we have made the userspace
and inkernel ABIs available (as they can often cause the interrupt to fire).
So I'd normally expect the iio_device_register call to be very last one
in probe. Why is it not in this case?
> +
> + return 0;
> +}
> +
> +static int mt6360_adc_remove(struct platform_device *pdev)
> +{
> + struct mt6360_adc_data *mad = platform_get_drvdata(pdev);
> +
> + if (mad->scan_task)
> + kthread_stop(mad->scan_task);
I'm a bit surprised this is needed. Remove should result in
iio_device_unregister being called which should smoothly shut
down any buffered capture that is ongoing and I would have
assumed would hence stop the thread.
You may have an ordering issue though.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused mt6360_adc_of_id[] = {
> + { .compatible = "mediatek,mt6360_adc", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mt6360_adc_of_id);
> +
> +static struct platform_driver mt6360_adc_driver = {
> + .driver = {
> + .name = "mt6360_adc",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(mt6360_adc_of_id),
Whilst it is fairly unlikely I guess that someone might want to use
ACPI to probe this lets not prevent it without good reason
(via the magic PRP0001) ID so please drop the of_match_ptr protection.
> + },
> + .probe = mt6360_adc_probe,
> + .remove = mt6360_adc_remove,
> +};
> +module_platform_driver(mt6360_adc_driver);
> +
> +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
> +MODULE_DESCRIPTION("MT6360 ADC Driver");
> +MODULE_LICENSE("GPL v2");
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] leds: mt6360: Add LED driver for MT6360
From: Dan Murphy @ 2020-06-04 16:07 UTC (permalink / raw)
To: Gene Chen, jacek.anaszewski, pavel, matthias.bgg
Cc: gene_chen, linux-kernel, cy_huang, benjamin.chao, linux-mediatek,
linux-leds, Wilma.Wu, linux-arm-kernel, shufan_lee
In-Reply-To: <1591252004-12681-1-git-send-email-gene.chen.richtek@gmail.com>
Gene
On 6/4/20 1:26 AM, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
>
> Add MT6360 LED driver include 2-channel Flash LED with torch/strobe mode,
> and 3-channel RGB LED support Register/Flash/Breath Mode
>
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> base-commit: 098c4adf249c198519a4abebe482b1e6b8c50e47
> ---
> drivers/leds/Kconfig | 11 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-mt6360.c | 1061 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/mt6360.h | 6 +-
> 4 files changed, 1078 insertions(+), 1 deletion(-)
> create mode 100644 drivers/leds/leds-mt6360.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index c664d84..c47be91 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -229,6 +229,17 @@ config LEDS_MT6323
> This option enables support for on-chip LED drivers found on
> Mediatek MT6323 PMIC.
>
> +config LEDS_MT6360
> + tristate "LED Support for Mediatek MT6360 PMIC"
> + depends on LEDS_CLASS_FLASH && OF
> + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> + depends on MFD_MT6360
> + help
> + This option enables support for dual Flash LED drivers found on
> + Mediatek MT6360 PMIC.
> + Support Torch and Strobe mode independently current source.
> + Include Low-VF and short protection.
> +
> config LEDS_S3C24XX
> tristate "LED Support for Samsung S3C24XX GPIO LEDs"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 45235d5..2883b4d 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -63,6 +63,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o
> obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o
> obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
> +obj-$(CONFIG_LEDS_MT6360) += leds-mt6360.o
> obj-$(CONFIG_LEDS_NET48XX) += leds-net48xx.o
> obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
> diff --git a/drivers/leds/leds-mt6360.c b/drivers/leds/leds-mt6360.c
> new file mode 100644
> index 0000000..3e62547
> --- /dev/null
> +++ b/drivers/leds/leds-mt6360.c
> @@ -0,0 +1,1061 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + *
> + * Author: Gene Chen <gene_chen@richtek.com>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/led-class-flash.h>
> +#include <media/v4l2-flash-led-class.h>
> +
> +#include <linux/mfd/mt6360.h>
> +
> +enum {
> + MT6360_LED_ISINK1 = 0,
> + MT6360_LED_ISINK2,
> + MT6360_LED_ISINK3,
> + MT6360_LED_ISINK4,
> + MT6360_LED_MAX,
> +};
> +
> +enum {
> + MT6360_LEDMODE_PWM = 0,
> + MT6360_LEDMODE_BREATH,
> + MT6360_LEDMODE_CC,
> + MT6360_LEDMODE_MAX,
> +};
> +
> +enum {
> + MT6360_FLED_CH1 = 0,
> + MT6360_FLED_CH2,
> + MT6360_FLED_MAX,
> +};
> +
> +/* ILED setting/reg */
> +#define MT6360_SINKCUR_MAX1 (0x0d)
> +#define MT6360_SINKCUR_MAX2 (0x0d)
> +#define MT6360_SINKCUR_MAX3 (0x0d)
> +#define MT6360_SINKCUR_MAX4 (0x1f)
> +#define MT6360_CURRSEL_REG1 (MT6360_PMU_RGB1_ISNK)
> +#define MT6360_CURRSEL_REG2 (MT6360_PMU_RGB2_ISNK)
> +#define MT6360_CURRSEL_REG3 (MT6360_PMU_RGB3_ISNK)
> +#define MT6360_CURRSEL_REG4 (MT6360_PMU_RGB_ML_ISNK)
> +#define MT6360_CURRSEL_MASK1 (0x0f)
> +#define MT6360_CURRSEL_MASK2 (0x0f)
> +#define MT6360_CURRSEL_MASK3 (0x0f)
> +#define MT6360_CURRSEL_MASK4 (0x1f)
> +#define MT6360_LEDMODE_REG1 (MT6360_PMU_RGB1_ISNK)
> +#define MT6360_LEDMODE_REG2 (MT6360_PMU_RGB2_ISNK)
> +#define MT6360_LEDMODE_REG3 (MT6360_PMU_RGB3_ISNK)
> +#define MT6360_LEDMODE_REG4 (0)
> +#define MT6360_LEDMODE_MASK1 (0xc0)
> +#define MT6360_LEDMODE_MASK2 (0xc0)
> +#define MT6360_LEDMODE_MASK3 (0xc0)
> +#define MT6360_LEDMODE_MASK4 (0)
> +#define MT6360_PWMDUTY_REG1 (MT6360_PMU_RGB1_DIM)
> +#define MT6360_PWMDUTY_REG2 (MT6360_PMU_RGB2_DIM)
> +#define MT6360_PWMDUTY_REG3 (MT6360_PMU_RGB3_DIM)
> +#define MT6360_PWMDUTY_REG4 (0)
> +#define MT6360_PWMDUTY_MASK1 (0xff)
> +#define MT6360_PWMDUTY_MASK2 (0xff)
> +#define MT6360_PWMDUTY_MASK3 (0xff)
> +#define MT6360_PWMDUTY_MASK4 (0)
> +#define MT6360_PWMFREQ_REG1 (MT6360_PMU_RGB12_Freq)
> +#define MT6360_PWMFREQ_REG2 (MT6360_PMU_RGB12_Freq)
> +#define MT6360_PWMFREQ_REG3 (MT6360_PMU_RGB34_Freq)
> +#define MT6360_PWMFREQ_REG4 (0)
> +#define MT6360_PWMFREQ_MASK1 (0xe0)
> +#define MT6360_PWMFREQ_MASK2 (0x1c)
> +#define MT6360_PWMFREQ_MASK3 (0xe0)
> +#define MT6360_PWMFREQ_MASK4 (0)
> +#define MT6360_BREATH_REGBASE1 (MT6360_PMU_RGB1_Tr)
> +#define MT6360_BREATH_REGBASE2 (MT6360_PMU_RGB2_Tr)
> +#define MT6360_BREATH_REGBASE3 (MT6360_PMU_RGB3_Tr)
> +#define MT6360_BREATH_REGBASE4 (0)
> +#define MT6360_LEDEN_MASK1 (0x80)
> +#define MT6360_LEDEN_MASK2 (0x40)
> +#define MT6360_LEDEN_MASK3 (0x20)
> +#define MT6360_LEDEN_MASK4 (0x10)
> +#define MT6360_LEDEN_REG (MT6360_PMU_RGB_EN)
> +#define MT6360_LEDALLEN_MASK (0xf0)
> +
> +#define MT6360_CHRIND_MASK (0x08)
> +
> +/* pattern order -> toff, tr1, tr2, ton, tf1, tf2 */
> +#define MT6360_BRPATTERN_NUM (6)
> +#define MT6360_BREATHREG_NUM (3)
> +
> +/* FLED setting */
> +#define MT6360_CSENABLE_REG1 (MT6360_PMU_FLED_EN)
> +#define MT6360_CSENABLE_MASK1 (0x02)
> +#define MT6360_CSENABLE_REG2 (MT6360_PMU_FLED_EN)
> +#define MT6360_CSENABLE_MASK2 (0x01)
> +#define MT6360_TORBRIGHT_MAX1 (0x1f)
> +#define MT6360_TORBRIGHT_MAX2 (0x1f)
> +#define MT6360_TORBRIGHT_REG1 (MT6360_PMU_FLED1_TOR_CTRL)
> +#define MT6360_TORBRIGHT_MASK1 (0x1f)
> +#define MT6360_STRBRIGHT_REG1 (MT6360_PMU_FLED1_STRB_CTRL2)
> +#define MT6360_STRBRIGHT_MASK1 (0x7f)
> +#define MT6360_TORBRIGHT_REG2 (MT6360_PMU_FLED2_TOR_CTRL)
> +#define MT6360_TORBRIGHT_MASK2 (0x1f)
> +#define MT6360_STRBRIGHT_REG2 (MT6360_PMU_FLED2_STRB_CTRL2)
> +#define MT6360_STRBRIGHT_MASK2 (0x7f)
> +#define MT6360_TORENABLE_REG1 (MT6360_PMU_FLED_EN)
> +#define MT6360_TORENABLE_MASK1 (0x08)
> +#define MT6360_TORENABLE_REG2 (MT6360_PMU_FLED_EN)
> +#define MT6360_TORENABLE_MASK2 (0x08)
> +#define MT6360_STRBENABLE_REG1 (MT6360_PMU_FLED_EN)
> +#define MT6360_STRBENABLE_MASK1 (0x06)
> +#define MT6360_STRBENABLE_REG2 (MT6360_PMU_FLED_EN)
> +#define MT6360_STRBENABLE_MASK2 (0x04)
> +#define MT6360_STRBTIMEOUT_REG (MT6360_PMU_FLED_STRB_CTRL)
> +#define MT6360_STRBTIMEOUT_MASK (0x7f)
> +#define MT6360_TORCHCUR_MIN (25000)
> +#define MT6360_TORCHCUR_STEP (12500)
> +#define MT6360_TORCHCUR_MAX (400000)
> +#define MT6360_STROBECUR_MIN (50000)
> +#define MT6360_STROBECUR_STEP (12500)
> +#define MT6360_STROBECUR_MAX (1500000)
> +#define MT6360_STRBTIMEOUT_MIN (64000)
> +#define MT6360_STRBTIMEOUT_STEP (32000)
> +#define MT6360_STRBTIMEOUT_MAX (2432000)
> +
> +#define MT6360_MASK_ULTRA_STROBE (0x80)
> +#define MT6360_SHIFT_ULTRA_STROBE (7)
> +
> +#define MT6360_FLEDSUPPORT_FAULTS (LED_FAULT_UNDER_VOLTAGE |\
> + LED_FAULT_SHORT_CIRCUIT |\
> + LED_FAULT_INPUT_VOLTAGE |\
> + LED_FAULT_TIMEOUT)
> +
> +struct mt6360_led_platform_data {
> + u32 rgbon_sync;
> + u32 fled1_ultraistrb;
> + u32 fled2_ultraistrb;
> +};
> +
> +struct breath_element_cfg {
> + /* base, step in ms */
> + unsigned int base;
> + unsigned int step;
> + unsigned int maxval;
> + unsigned int reg_offset;
> + unsigned int reg_mask;
> +};
> +
> +struct mt6360_led_classdev {
> + struct led_classdev cdev;
> + int index;
> + struct device_node *np;
> + unsigned int currsel_reg;
> + unsigned int currsel_mask;
> + unsigned int enable_mask;
> + unsigned int mode_reg;
> + unsigned int mode_mask;
> + unsigned int pwmduty_reg;
> + unsigned int pwmduty_mask;
> + unsigned int pwmfreq_reg;
> + unsigned int pwmfreq_mask;
> + unsigned int breath_regbase;
> +};
> +
> +struct mt6360_fled_classdev {
> + struct led_classdev_flash fl_cdev;
> + int index;
> + struct v4l2_flash *v4l2_flash;
> + struct device_node *np;
> + unsigned int cs_enable_reg;
> + unsigned int cs_enable_mask;
> + unsigned int torch_bright_reg;
> + unsigned int torch_bright_mask;
> + unsigned int torch_enable_reg;
> + unsigned int torch_enable_mask;
> + unsigned int strobe_bright_reg;
> + unsigned int strobe_bright_mask;
> + unsigned int strobe_enable_reg;
> + unsigned int strobe_enable_mask;
> + unsigned int strobe_external_reg;
> + unsigned int strobe_external_mask;
> + u32 faults;
> +};
> +
> +struct mt6360_led_data {
> + struct device *dev;
> + struct mt6360_led_platform_data *pdata;
> + struct regmap *regmap;
> + struct mt6360_led_classdev mtled_cdev[MT6360_LED_MAX];
> + struct mt6360_fled_classdev mtfled_cdev[MT6360_FLED_MAX];
> + unsigned long fl_torch_flags;
> + unsigned long fl_strobe_flags;
> +};
> +
> +static const struct mt6360_led_platform_data def_platform_data = {
> + .rgbon_sync = 0,
> + .fled1_ultraistrb = 1,
> + .fled2_ultraistrb = 1,
> +};
> +
> +static int mt6360_led_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct mt6360_led_classdev *mtled_cdev =
> + (struct mt6360_led_classdev *)cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
> + int shift, sync_regval = 0, ret;
> +
> + /* if isink1 user control, set chrind function to sw mode */
> + if (mtled_cdev->index == MT6360_LED_ISINK1) {
> + ret = regmap_update_bits(mld->regmap,
> + MT6360_PMU_RGB_EN, MT6360_CHRIND_MASK, 0xff);
> + if (ret < 0)
> + dev_err(cdev->dev, "disable chrind func fail\n");
> + }
> + if (brightness == LED_OFF) {
> + ret = regmap_update_bits(mld->regmap,
> + MT6360_LEDEN_REG, mtled_cdev->enable_mask, 0);
> + if (ret < 0)
> + return ret;
> + if (mtled_cdev->mode_reg == 0)
> + goto out_bright_set;
> + /* if off, force config to cc_mode */
> + shift = ffs(mtled_cdev->mode_mask) - 1;
> + ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
> + mtled_cdev->mode_mask, MT6360_LEDMODE_CC << shift);
> + if (ret < 0)
> + dev_err(cdev->dev, "config cc mode fail\n");
> + goto out_bright_set;
> + }
> + shift = ffs(mtled_cdev->currsel_mask) - 1;
> + brightness -= 1;
> + ret = regmap_update_bits(mld->regmap, mtled_cdev->currsel_reg,
> + mtled_cdev->currsel_mask, brightness << shift);
> + if (ret < 0)
> + return ret;
> + if (mld->pdata->rgbon_sync) {
> + ret = regmap_read(mld->regmap, MT6360_LEDEN_REG, &sync_regval);
> + if (ret < 0)
> + goto out_bright_set;
> + ret = regmap_update_bits(mld->regmap,
> + MT6360_LEDEN_REG, MT6360_LEDALLEN_MASK, 0);
> + if (ret < 0)
> + goto out_bright_set;
> + sync_regval |= mtled_cdev->enable_mask;
> + ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
> + MT6360_LEDALLEN_MASK, sync_regval);
> + } else {
> + ret = regmap_update_bits(mld->regmap, MT6360_LEDEN_REG,
> + mtled_cdev->enable_mask, 0xff);
> + }
> +out_bright_set:
> + return ret;
> +}
> +
> +static enum led_brightness mt6360_led_brightness_get(struct led_classdev *cdev)
> +{
> + struct mt6360_led_classdev *mtled_cdev =
> + (struct mt6360_led_classdev *)cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
> + unsigned int regval = 0;
> + int shift = ffs(mtled_cdev->currsel_mask) - 1, ret;
> +
> + ret = regmap_read(mld->regmap, MT6360_LEDEN_REG, ®val);
> + if (ret < 0) {
> + dev_err(cdev->dev, "%s: get enable fail\n", __func__);
> + return LED_OFF;
> + }
> + if (!(regval & mtled_cdev->enable_mask))
> + return LED_OFF;
> + ret = regmap_read(mld->regmap, mtled_cdev->currsel_reg, ®val);
> + if (ret < 0) {
> + dev_err(cdev->dev, "%s: get isink fail\n", __func__);
> + return LED_OFF;
> + }
> + regval &= mtled_cdev->currsel_mask;
> + regval >>= shift;
> + return (regval + 1);
> +}
> +
> +static const unsigned int dim_freqs[] = {
> + 4, 8, 250, 500, 1000, 2000, 4000, 8000,
> +};
> +
> +static int mt6360_led_blink_set(struct led_classdev *cdev,
> + unsigned long *delay_on, unsigned long *delay_off)
> +{
> + struct mt6360_led_classdev *mtled_cdev =
> + (struct mt6360_led_classdev *)cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(cdev->dev->parent);
> + int freq, duty, shift, sum, ret;
> +
> + if (mtled_cdev->mode_reg == 0)
> + return -ENOTSUPP;
> + if (*delay_on == 0 && *delay_off == 0)
> + *delay_on = *delay_off = 500;
> + sum = *delay_on + *delay_off;
> + for (freq = 0; freq < ARRAY_SIZE(dim_freqs); freq++) {
> + if (sum <= dim_freqs[freq])
> + break;
> + }
> + if (freq == ARRAY_SIZE(dim_freqs)) {
> + dev_err(cdev->dev, "exceed pwm frequency max\n");
> + return -EINVAL;
> + }
> + /* invert */
> + freq = ARRAY_SIZE(dim_freqs) - 1 - freq;
> + shift = ffs(mtled_cdev->pwmfreq_mask) - 1;
> + ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmfreq_reg,
> + mtled_cdev->pwmfreq_mask, freq << shift);
> + if (ret < 0) {
> + dev_err(cdev->dev, "Failed to set pwmfreq\n");
> + return ret;
> + }
> + duty = 255 * (*delay_on) / sum;
> + shift = ffs(mtled_cdev->pwmduty_mask) - 1;
> + ret = regmap_update_bits(mld->regmap, mtled_cdev->pwmduty_reg,
> + mtled_cdev->pwmduty_mask, duty << shift);
> + if (ret < 0) {
> + dev_err(cdev->dev, "Failed to set pwmduty\n");
> + return ret;
> + }
> + shift = ffs(mtled_cdev->mode_mask) - 1;
> + ret = regmap_update_bits(mld->regmap, mtled_cdev->mode_reg,
> + mtled_cdev->mode_mask, MT6360_LEDMODE_PWM << shift);
> + return ret;
> +}
> +
> +#define MT6360_LED_DESC(_id) { \
> + .cdev = { \
> + .name = "mt6360_isink" #_id, \
> + .max_brightness = MT6360_SINKCUR_MAX##_id, \
> + .brightness_set_blocking = mt6360_led_brightness_set, \
> + .brightness_get = mt6360_led_brightness_get, \
> + .blink_set = mt6360_led_blink_set, \
> + }, \
> + .index = MT6360_LED_ISINK##_id, \
> + .currsel_reg = MT6360_CURRSEL_REG##_id, \
> + .currsel_mask = MT6360_CURRSEL_MASK##_id, \
> + .enable_mask = MT6360_LEDEN_MASK##_id, \
> + .mode_reg = MT6360_LEDMODE_REG##_id, \
> + .mode_mask = MT6360_LEDMODE_MASK##_id, \
> + .pwmduty_reg = MT6360_PWMDUTY_REG##_id, \
> + .pwmduty_mask = MT6360_PWMDUTY_MASK##_id, \
> + .pwmfreq_reg = MT6360_PWMFREQ_REG##_id, \
> + .pwmfreq_mask = MT6360_PWMFREQ_MASK##_id, \
> + .breath_regbase = MT6360_BREATH_REGBASE##_id, \
> +}
> +
> +/* ISINK 1/2/3 for RGBLED, ISINK4 for MoonLight */
> +static const struct mt6360_led_classdev def_led_classdev[MT6360_LED_MAX] = {
> + MT6360_LED_DESC(1),
> + MT6360_LED_DESC(2),
> + MT6360_LED_DESC(3),
> + MT6360_LED_DESC(4),
> +};
> +
> +static inline bool mt6360_fled_check_flags_if_any(unsigned long *flags)
> +{
> + return (*flags) ? true : false;
> +}
> +
> +static int mt6360_fled_strobe_brightness_set(
> + struct led_classdev_flash *fled_cdev, u32 brightness)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct led_flash_setting *fs = &fled_cdev->brightness;
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> + int shift;
> + u32 val;
> +
> + val = brightness;
> + val = (val - fs->min) / fs->step;
> + shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
> + return regmap_update_bits(mld->regmap, mtfled_cdev->strobe_bright_reg,
> + mtfled_cdev->strobe_bright_mask, val << shift);
> +}
> +
> +static int mt6360_fled_strobe_brightness_get(
> + struct led_classdev_flash *fled_cdev, u32 *brightness)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct led_flash_setting *fs = &fled_cdev->brightness;
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> + int shift, ret;
> + u32 regval = 0;
> +
> + ret = regmap_read(mld->regmap, mtfled_cdev->strobe_bright_reg, ®val);
> + if (ret < 0)
> + return ret;
> + regval &= mtfled_cdev->strobe_bright_mask;
> + shift = ffs(mtfled_cdev->strobe_bright_mask) - 1;
> + regval >>= shift;
> + /* convert to microamp value */
> + *brightness = regval * fs->step + fs->min;
> + return 0;
> +}
> +
> +static int mt6360_fled_strobe_set(
> + struct led_classdev_flash *fled_cdev, bool state)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> + int id = mtfled_cdev->index, ret;
> +
> + if (!(state ^ test_bit(id, &mld->fl_strobe_flags)))
> + return 0;
> + if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
> + dev_err(led_cdev->dev,
> + "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
> + return -EINVAL;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, state ? 0xff : 0);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set cs enable [%d]\n", state);
> + goto out_strobe_set;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->strobe_enable_reg,
> + mtfled_cdev->strobe_enable_mask, state ? 0xff : 0);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set strb enable [%d]\n", state);
> + goto out_strobe_set;
> + }
> + if (state) {
> + if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
> + usleep_range(5000, 6000);
> + set_bit(id, &mld->fl_strobe_flags);
> + mtfled_cdev->faults = 0;
> + } else {
> + clear_bit(id, &mld->fl_strobe_flags);
> + if (!mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags))
> + usleep_range(400, 500);
> + }
> +out_strobe_set:
> + return ret;
> +}
> +
> +static int mt6360_fled_strobe_get(
> + struct led_classdev_flash *fled_cdev, bool *state)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> + int id = mtfled_cdev->index;
> +
> + *state = test_bit(id, &mld->fl_strobe_flags) ? true : false;
> + return 0;
> +}
> +
> +static int mt6360_fled_strobe_timeout_set(
> + struct led_classdev_flash *fled_cdev, u32 timeout)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct led_flash_setting *ts = &fled_cdev->timeout;
> + int shift, ret;
> + u32 regval;
> +
> + regval = (timeout - ts->min) / ts->step;
> + shift = ffs(MT6360_STRBTIMEOUT_MASK) - 1;
> + ret = regmap_update_bits(mld->regmap, MT6360_STRBTIMEOUT_REG,
> + MT6360_STRBTIMEOUT_MASK, regval << shift);
> + return ret;
> +}
> +
> +static int mt6360_fled_strobe_fault_get(
> + struct led_classdev_flash *fled_cdev, u32 *fault)
> +{
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> +
> + *fault = mtfled_cdev->faults;
> + return 0;
> +}
> +
> +static const struct led_flash_ops mt6360_fled_ops = {
> + .flash_brightness_set = mt6360_fled_strobe_brightness_set,
> + .flash_brightness_get = mt6360_fled_strobe_brightness_get,
> + .strobe_set = mt6360_fled_strobe_set,
> + .strobe_get = mt6360_fled_strobe_get,
> + .timeout_set = mt6360_fled_strobe_timeout_set,
> + .fault_get = mt6360_fled_strobe_fault_get,
> +};
> +
> +static int mt6360_fled_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
> + int id = mtfled_cdev->index, shift, keep, ret;
> +
> + if (mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags)) {
> + dev_err(led_cdev->dev,
> + "Disable all leds strobe [%lu]\n", mld->fl_strobe_flags);
> + return -EINVAL;
> + }
> + if (brightness == LED_OFF) {
> + clear_bit(id, &mld->fl_torch_flags);
> + keep = mt6360_fled_check_flags_if_any(&mld->fl_torch_flags);
> + ret = regmap_update_bits(mld->regmap,
> + mtfled_cdev->torch_enable_reg,
> + mtfled_cdev->torch_enable_mask,
> + keep ? 0xff : 0);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set torch disable\n");
> + goto out_bright_set;
> + }
> + ret = regmap_update_bits(mld->regmap,
> + mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, 0);
> + if (ret < 0)
> + dev_err(led_cdev->dev, "Fail to set torch disable\n");
> + goto out_bright_set;
> + }
> + shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
> + brightness -= 1;
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_bright_reg,
> + mtfled_cdev->torch_bright_mask, brightness << shift);
> + if (ret < 0) {
> + dev_err(led_cdev->dev,
> + "Fail to set torch bright [%d]\n", brightness);
> + goto out_bright_set;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, 0xff);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set cs enable\n");
> + goto out_bright_set;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->torch_enable_reg,
> + mtfled_cdev->torch_enable_mask, 0xff);
> + set_bit(id, &mld->fl_torch_flags);
> +out_bright_set:
> + return ret;
> +}
> +
> +static enum led_brightness mt6360_fled_brightness_get(
> + struct led_classdev *led_cdev)
> +{
> + struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
> + int id = mtfled_cdev->index, shift, ret;
> + u32 regval = 0;
> +
> + if (!test_bit(id, &mld->fl_torch_flags))
> + return LED_OFF;
> + ret = regmap_read(mld->regmap, mtfled_cdev->torch_bright_reg, ®val);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "%s: Fail to get torb reg\n", __func__);
> + return LED_OFF;
> + }
> + shift = ffs(mtfled_cdev->torch_bright_mask) - 1;
> + regval &= mtfled_cdev->torch_bright_mask;
> + regval >>= shift;
> + return (regval + 1);
> +}
> +
> +#define MT6360_FLED_DESC(_id) { \
> + .fl_cdev = { \
> + .led_cdev = { \
> + .name = "mt6360_fled_ch" #_id, \
> + .max_brightness = MT6360_TORBRIGHT_MAX##_id, \
> + .brightness_set_blocking = mt6360_fled_brightness_set, \
> + .brightness_get = mt6360_fled_brightness_get, \
> + .flags = LED_DEV_CAP_FLASH, \
> + }, \
> + .brightness = { \
> + .min = MT6360_STROBECUR_MIN, \
> + .step = MT6360_STROBECUR_STEP, \
> + .max = MT6360_STROBECUR_MAX, \
> + .val = MT6360_STROBECUR_MIN, \
> + }, \
> + .timeout = { \
> + .min = MT6360_STRBTIMEOUT_MIN, \
> + .step = MT6360_STRBTIMEOUT_STEP, \
> + .max = MT6360_STRBTIMEOUT_MAX, \
> + .val = MT6360_STRBTIMEOUT_MIN, \
> + }, \
> + .ops = &mt6360_fled_ops, \
> + }, \
> + .index = MT6360_FLED_CH##_id, \
> + .cs_enable_reg = MT6360_CSENABLE_REG##_id, \
> + .cs_enable_mask = MT6360_CSENABLE_MASK##_id, \
> + .torch_bright_reg = MT6360_TORBRIGHT_REG##_id, \
> + .torch_bright_mask = MT6360_TORBRIGHT_MASK##_id, \
> + .torch_enable_reg = MT6360_TORENABLE_REG##_id, \
> + .torch_enable_mask = MT6360_TORENABLE_MASK##_id, \
> + .strobe_bright_reg = MT6360_STRBRIGHT_REG##_id, \
> + .strobe_bright_mask = MT6360_STRBRIGHT_MASK##_id, \
> + .strobe_enable_reg = MT6360_STRBENABLE_REG##_id, \
> + .strobe_enable_mask = MT6360_STRBENABLE_MASK##_id, \
> +}
> +
> +static const struct mt6360_fled_classdev def_fled_classdev[MT6360_FLED_MAX] = {
> + MT6360_FLED_DESC(1),
> + MT6360_FLED_DESC(2),
> +};
> +
> +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
> +static int mt6360_fled_external_strobe_set(
> + struct v4l2_flash *v4l2_flash, bool enable)
> +{
> + struct led_classdev_flash *lcf = v4l2_flash->fled_cdev;
> + struct led_classdev *led_cdev = &lcf->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
> + int id = mtfled_cdev->index, ret;
> +
> + if (!(enable ^ test_bit(id, &mld->fl_strobe_flags)))
> + return 0;
> + if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
> + dev_err(led_cdev->dev,
> + "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
> + return -EINVAL;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, enable ? 0xff : 0);
> + if (enable) {
> + set_bit(id, &mld->fl_strobe_flags);
> + mtfled_cdev->faults = 0;
> + } else
> + clear_bit(id, &mld->fl_strobe_flags);
> + return ret;
> +}
> +
> +static const struct v4l2_flash_ops v4l2_flash_ops = {
> + .external_strobe_set = mt6360_fled_external_strobe_set,
> +};
> +
> +static void mt6360_init_v4l2_flash_config(
> + struct mt6360_fled_classdev *mtfled_cdev,
> + struct v4l2_flash_config *config)
> +{
> + struct led_flash_setting *torch_intensity = &config->intensity;
> + struct led_classdev *led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
> + s32 val;
> +
> + snprintf(config->dev_name, sizeof(config->dev_name),
> + "%s", mtfled_cdev->fl_cdev.led_cdev.name);
> + torch_intensity->min = MT6360_TORCHCUR_MIN;
> + torch_intensity->step = MT6360_TORCHCUR_STEP;
> + val = MT6360_TORCHCUR_MIN;
> + val += ((led_cdev->max_brightness - 1) * MT6360_TORCHCUR_STEP);
> + torch_intensity->val = torch_intensity->max = val;
> + config->flash_faults |= MT6360_FLEDSUPPORT_FAULTS;
> + config->has_external_strobe = 1;
> +}
> +#else
> +static const struct v4l2_flash_ops v4l2_flash_ops;
> +
> +static void mt6360_init_v4l2_flash_config(
> + struct mt6360_fled_classdev *mtfled_cdev,
> + struct v4l2_flash_config *config)
> +{
> +}
> +#endif /* IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS) */
> +
> +static irqreturn_t mt6360_pmu_fled_lvf_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + dev_err(mld->dev, "%s\n", __func__);
> + mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_UNDER_VOLTAGE;
> + mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_UNDER_VOLTAGE;
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mt6360_pmu_fled2_short_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + dev_err(mld->dev, "%s\n", __func__);
> + mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_SHORT_CIRCUIT;
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mt6360_pmu_fled1_short_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + dev_err(mld->dev, "%s\n", __func__);
> + mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_SHORT_CIRCUIT;
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mt6360_pmu_fled2_strb_to_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_TIMEOUT;
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mt6360_pmu_fled1_strb_to_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_TIMEOUT;
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t mt6360_pmu_fled_chg_vinovp_evt_handler(int irq, void *data)
> +{
> + struct mt6360_led_data *mld = data;
> +
> + dev_warn(mld->dev, "%s\n", __func__);
> + mld->mtfled_cdev[MT6360_FLED_CH1].faults |= LED_FAULT_INPUT_VOLTAGE;
> + mld->mtfled_cdev[MT6360_FLED_CH2].faults |= LED_FAULT_INPUT_VOLTAGE;
> + return IRQ_HANDLED;
> +}
> +
> +static struct mt6360_pmu_irq_desc mt6360_pmu_fled_irq_desc[] = {
> + { "fled_chg_vinovp_evt", mt6360_pmu_fled_chg_vinovp_evt_handler },
> + { "fled_lvf_evt", mt6360_pmu_fled_lvf_evt_handler },
> + { "fled2_short_evt", mt6360_pmu_fled2_short_evt_handler },
> + { "fled1_short_evt", mt6360_pmu_fled1_short_evt_handler },
> + { "fled2_strb_to_evt", mt6360_pmu_fled2_strb_to_evt_handler },
> + { "fled1_strb_to_evt", mt6360_pmu_fled1_strb_to_evt_handler },
> +};
> +
> +static int mt6360_fled_irq_register(struct platform_device *pdev)
> +{
> + struct mt6360_pmu_irq_desc *irq_desc;
> + int i, irq, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(mt6360_pmu_fled_irq_desc); i++) {
> + irq_desc = mt6360_pmu_fled_irq_desc + i;
> + if (unlikely(!irq_desc->name))
> + continue;
> + irq = platform_get_irq_byname(pdev, irq_desc->name);
> + if (irq < 0)
> + continue;
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + irq_desc->irq_handler,
> + IRQF_TRIGGER_FALLING,
> + irq_desc->name,
> + platform_get_drvdata(pdev));
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "request %s irq fail\n", irq_desc->name);
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> +static int mt6360_iled_parse_dt(struct device *dev,
> + struct mt6360_led_data *mld)
> +{
> + struct device_node *iled_np, *child;
> + struct mt6360_led_classdev *mtled_cdev;
> + u32 val;
> + int ret;
> +
> + if (!dev->of_node)
> + return 0;
> + iled_np = of_find_node_by_name(dev->of_node, "iled");
This should really follow the LED node naming convention
> + if (!iled_np)
> + return 0;
> + for_each_available_child_of_node(iled_np, child) {
Please use fwnode APIs
> + ret = of_property_read_u32(child, "reg", &val);
> + if (ret) {
> + dev_err(dev, "Fail to read reg property\n");
> + continue;
> + }
> + if (val >= MT6360_LED_MAX) {
> + dev_err(dev, "Invalid iled reg [%u]\n", val);
> + ret = -EINVAL;
> + goto out_iled_dt;
> + }
> + mtled_cdev = mld->mtled_cdev + val;
> +
> + of_property_read_string(child,
> + "label", &(mtled_cdev->cdev.name));
> + of_property_read_string(child, "linux,default-trigger",
> + &(mtled_cdev->cdev.default_trigger));
Same here
> + mtled_cdev->np = child;
> + }
> + return 0;
> +out_iled_dt:
> + of_node_put(child);
> + return ret;
> +}
> +
> +static int mt6360_fled_parse_dt(struct device *dev,
> + struct mt6360_led_data *mld)
> +{
> + struct device_node *fled_np, *child;
> + struct mt6360_fled_classdev *mtfled_cdev;
> + struct led_classdev *led_cdev;
> + struct led_flash_setting *fs;
> + u32 val;
> + int ret;
> +
> + if (!dev->of_node)
> + return 0;
Is this really necessary can this be done once by the caller before
calling each parse_dt function?
> + fled_np = of_find_node_by_name(dev->of_node, "fled");
> + if (!fled_np)
> + return 0;
> + for_each_available_child_of_node(fled_np, child) {
> + ret = of_property_read_u32(child, "reg", &val);
> + if (ret) {
> + dev_err(dev, "Fail to read reg property\n");
> + continue;
> + }
> + if (val >= MT6360_FLED_MAX) {
> + dev_err(dev, "Invalid fled reg [%u]\n", val);
> + ret = -EINVAL;
> + goto out_fled_dt;
> + }
> + mtfled_cdev = mld->mtfled_cdev + val;
> +
> + of_property_read_string(child, "label",
> + &(mtfled_cdev->fl_cdev.led_cdev.name));
> + ret = of_property_read_u32(child, "led-max-microamp", &val);
> + if (ret) {
> + dev_warn(dev, "led-max-microamp property missing\n");
> + val = MT6360_TORCHCUR_MIN;
> + }
> + if (val < MT6360_TORCHCUR_MIN)
> + val = MT6360_TORCHCUR_MIN;
> + val = (val - MT6360_TORCHCUR_MIN) / MT6360_TORCHCUR_STEP + 1;
> + led_cdev = &(mtfled_cdev->fl_cdev.led_cdev);
> + led_cdev->max_brightness = min(led_cdev->max_brightness, val);
> + ret = of_property_read_u32(child, "flash-max-microamp", &val);
> + if (ret) {
> + dev_warn(dev, "flash-max-microamp property missing\n");
> + val = MT6360_STROBECUR_MIN;
> + }
> + if (val < MT6360_STROBECUR_MIN)
> + val = MT6360_STROBECUR_MIN;
> + fs = &(mtfled_cdev->fl_cdev.brightness);
> + fs->val = fs->max = min(fs->max, val);
> + ret = of_property_read_u32(child, "flash-max-timeout", &val);
> + if (ret) {
> + dev_warn(dev, "flash-max-timeout property missing\n");
> + val = MT6360_STRBTIMEOUT_MIN;
> + }
> + if (val < MT6360_STRBTIMEOUT_MIN)
> + val = MT6360_STRBTIMEOUT_MIN;
> + fs = &(mtfled_cdev->fl_cdev.timeout);
> + fs->val = fs->max = min(fs->max, val);
> + mtfled_cdev->np = child;
> + }
> + return 0;
> +out_fled_dt:
> + of_node_put(child);
> + return ret;
> +}
> +
> +static void mt6360_led_parse_dt_data(struct device *dev,
> + struct mt6360_led_platform_data *pdata)
> +{
> + struct device_node *np = dev->of_node;
> + int i, ret;
> + const struct {
> + const char *name;
> + u32 *val_ptr;
> + } u32_opts[] = {
> + { "rgbon_sync", &pdata->rgbon_sync },
> + { "fled1_ultraistrb", &pdata->fled1_ultraistrb },
> + { "fled2_ultraistrb", &pdata->fled2_ultraistrb },
> + };
> +
> + memcpy(pdata, &def_platform_data, sizeof(*pdata));
> + for (i = 0; i < ARRAY_SIZE(u32_opts); i++) {
> + ret = of_property_read_u32(np, u32_opts[i].name,
> + u32_opts[i].val_ptr);
> + if (ret)
> + dev_err(dev, "error reading '%s'\n", u32_opts[i].name);
> + }
> +}
> +
> +static int mt6360_led_apply_pdata(struct mt6360_led_data *mld,
> + struct mt6360_led_platform_data *pdata)
> +{
> + int i, ret;
> + const struct {
> + u32 *val_ptr;
> + u8 reg;
> + u8 mask;
> + u8 shift;
> + } sel_props[] = {
> + {
> + &pdata->fled1_ultraistrb, MT6360_PMU_FLED1_STRB_CTRL2,
> + MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
> + },
> + {
> + &pdata->fled2_ultraistrb, MT6360_PMU_FLED2_STRB_CTRL2,
> + MT6360_MASK_ULTRA_STROBE, MT6360_SHIFT_ULTRA_STROBE,
> + },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(sel_props); i++) {
> + ret = regmap_update_bits(mld->regmap,
> + sel_props[i].reg,
> + sel_props[i].mask,
> + *sel_props[i].val_ptr <<
> + sel_props[i].shift);
> + if (ret < 0)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int mt6360_led_probe(struct platform_device *pdev)
> +{
> + struct mt6360_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct mt6360_led_data *mld;
> + struct mt6360_led_classdev *mtled_cdev;
> + struct mt6360_fled_classdev *mtfled_cdev;
> + struct v4l2_flash_config v4l2_config;
> + int i, ret;
> +
> + mld = devm_kzalloc(&pdev->dev, sizeof(*mld), GFP_KERNEL);
> + if (!mld)
> + return -ENOMEM;
> +
> + if (pdev->dev.of_node) {
> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> + mt6360_led_parse_dt_data(&pdev->dev, pdata);
> + }
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "no platform data specified\n");
> + return -EINVAL;
> + }
> +
> + mld->dev = &pdev->dev;
> + mld->pdata = pdata;
> + platform_set_drvdata(pdev, mld);
> +
> + mld->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!mld->regmap) {
> + dev_err(&pdev->dev, "Failed to get parent regmap\n");
> + return -ENODEV;
> + }
> +
> + ret = mt6360_led_apply_pdata(mld, pdata);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "apply pdata fail\n");
> + return ret;
> + }
> +
> + memcpy(mld->mtled_cdev, def_led_classdev, sizeof(def_led_classdev));
> + ret = mt6360_iled_parse_dt(&pdev->dev, mld);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to parse iled dt\n");
> + return ret;
> + }
> +
> + for (i = 0; i < MT6360_LED_MAX; i++) {
> + mtled_cdev = mld->mtled_cdev + i;
> + ret = devm_led_classdev_register(&pdev->dev,
> + &(mtled_cdev->cdev));
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register led[%d]\n", i);
> + return ret;
> + }
> + mtled_cdev->cdev.dev->of_node = mtled_cdev->np;
> + }
> +
> + memcpy(mld->mtfled_cdev, def_fled_classdev, sizeof(def_fled_classdev));
> + ret = mt6360_fled_parse_dt(&pdev->dev, mld);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Fail to parse fled dt\n");
> + return ret;
> + }
> +
> + for (i = 0; i < MT6360_FLED_MAX; i++) {
> + mtfled_cdev = mld->mtfled_cdev + i;
> + ret = led_classdev_flash_register(&pdev->dev,
> + &mtfled_cdev->fl_cdev);
use devm_* function
Dan
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register fled[%d]\n", i);
> + goto out_fled_cdev;
> + }
> + }
> +
> + for (i = 0; i < MT6360_FLED_MAX; i++) {
> + mtfled_cdev = mld->mtfled_cdev + i;
> + memset(&v4l2_config, 0, sizeof(v4l2_config));
> + mt6360_init_v4l2_flash_config(mtfled_cdev, &v4l2_config);
> + mtfled_cdev->v4l2_flash = v4l2_flash_init(&pdev->dev,
> + of_fwnode_handle(mtfled_cdev->np),
> + &mtfled_cdev->fl_cdev,
> + &v4l2_flash_ops, &v4l2_config);
> + if (IS_ERR(mtfled_cdev->v4l2_flash)) {
> + dev_err(&pdev->dev, "Failed to register v4l2_sd\n");
> + ret = PTR_ERR(mtfled_cdev->v4l2_flash);
> + goto out_v4l2_sd;
> + }
> + }
> +
> + ret = mt6360_fled_irq_register(pdev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to register irqs\n");
> + goto out_v4l2_sd;
> + }
> +
> + return 0;
> +out_v4l2_sd:
> + while (--i >= 0) {
> + mtfled_cdev = mld->mtfled_cdev + i;
> + v4l2_flash_release(mtfled_cdev->v4l2_flash);
> + }
> + i = MT6360_FLED_MAX;
> +out_fled_cdev:
> + while (--i >= 0) {
> + mtfled_cdev = mld->mtfled_cdev + i;
> + led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
> + }
> + return ret;
> +}
> +
> +static int mt6360_led_remove(struct platform_device *pdev)
> +{
> + struct mt6360_led_data *mld = platform_get_drvdata(pdev);
> + struct mt6360_fled_classdev *mtfled_cdev;
> + int i;
> +
> + for (i = 0; i < MT6360_FLED_MAX; i++) {
> + mtfled_cdev = mld->mtfled_cdev + i;
> + v4l2_flash_release(mtfled_cdev->v4l2_flash);
> + led_classdev_flash_unregister(&mtfled_cdev->fl_cdev);
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id __maybe_unused mt6360_led_of_id[] = {
> + { .compatible = "mediatek,mt6360_led", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mt6360_led_of_id);
> +
> +static struct platform_driver mt6360_led_driver = {
> + .driver = {
> + .name = "mt6360_led",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(mt6360_led_of_id),
> + },
> + .probe = mt6360_led_probe,
> + .remove = mt6360_led_remove,
> +};
> +module_platform_driver(mt6360_led_driver);
> +
> +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
> +MODULE_DESCRIPTION("MT6360 Led Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/mt6360.h b/include/linux/mfd/mt6360.h
> index ea13040..2b81ab5 100644
> --- a/include/linux/mfd/mt6360.h
> +++ b/include/linux/mfd/mt6360.h
> @@ -29,6 +29,11 @@ struct mt6360_pmu_data {
> unsigned int chip_rev;
> };
>
> +struct mt6360_pmu_irq_desc {
> + const char *name;
> + irq_handler_t irq_handler;
> +};
> +
> /* PMU register defininition */
> #define MT6360_PMU_DEV_INFO (0x00)
> #define MT6360_PMU_CORE_CTRL1 (0x01)
> @@ -236,5 +241,4 @@ struct mt6360_pmu_data {
> #define CHIP_VEN_MASK (0xF0)
> #define CHIP_VEN_MT6360 (0x50)
> #define CHIP_REV_MASK (0x0F)
> -
> #endif /* __MT6360_H__ */
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
From: David Hildenbrand @ 2020-06-04 15:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: wsd_upstream, Joerg Roedel, iommu, linux-kernel, Chao Hao,
Miles Chen, linux-mediatek, linux-arm-kernel, Matthias Brugger,
yingjoe.chen, Yong Wu
In-Reply-To: <20200604150643.GA29193@infradead.org>
On 04.06.20 17:06, Christoph Hellwig wrote:
> On Thu, Jun 04, 2020 at 01:32:40PM +0200, David Hildenbrand wrote:
>> Just a thought: If memory hotplug is applicable as well, you might
>> either want to always assume data->enable_4GB, or handle memory hotplug
>> events from the memory notifier, when new memory gets onlined (not sure
>> how tricky that is).
>
> We probably want a highest_pfn_possible() or similar API instead of
> having drivers poking into random VM internals.
Well, memory notifiers are a reasonable api used accross the kernel to
get notified when new memory is onlined to the buddy that could be used
for allocations.
highest_pfn_possible() would have to default to something linked to
MAX_PHYSMEM_BITS whenever memory hotplug is configured, I am not sure
how helpful that is (IOW, you can just default to enable_4GB=true in
that case instead in most cases).
--
Thanks,
David / dhildenb
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
From: Christoph Hellwig @ 2020-06-04 15:06 UTC (permalink / raw)
To: David Hildenbrand
Cc: wsd_upstream, Joerg Roedel, iommu, linux-kernel, Chao Hao,
Miles Chen, linux-mediatek, linux-arm-kernel, Matthias Brugger,
yingjoe.chen, Yong Wu
In-Reply-To: <f02c8c9d-ed75-6513-f8a9-a2fdbb11b750@redhat.com>
On Thu, Jun 04, 2020 at 01:32:40PM +0200, David Hildenbrand wrote:
> Just a thought: If memory hotplug is applicable as well, you might
> either want to always assume data->enable_4GB, or handle memory hotplug
> events from the memory notifier, when new memory gets onlined (not sure
> how tricky that is).
We probably want a highest_pfn_possible() or similar API instead of
having drivers poking into random VM internals.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] regulator: mt6360: Add support for MT6360 regulator
From: Randy Dunlap @ 2020-06-04 15:06 UTC (permalink / raw)
To: Gene Chen, matthias.bgg
Cc: gene_chen, lgirdwood, linux-kernel, cy_huang, benjamin.chao,
broonie, linux-mediatek, Wilma.Wu, linux-arm-kernel, shufan_lee
In-Reply-To: <1591254387-10304-1-git-send-email-gene.chen.richtek@gmail.com>
On 6/4/20 12:06 AM, Gene Chen wrote:
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index f4b72cb..05a3b14 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -680,6 +680,16 @@ config REGULATOR_MT6358
> This driver supports the control of different power rails of device
> through regulator interface.
>
> +config REGULATOR_MT6360
> + tristate "MT6360 SubPMIC Regulator"
> + depends on MFD_MT6360
> + select CRC8
> + help
> + Say Y here to enable MT6360 regulator support.
> + This is support MT6360 PMIC/LDO part include
This supports the MT6300 PMIC/LDO part, which includes
> + 2-channel buck with Thermal Shutdown and Overlord Protection
is that Overload ?
Yes, it could be Overlord.
> + 6-channel High PSRR and Low Dropout LDO.
> +
> config REGULATOR_MT6380
> tristate "MediaTek MT6380 PMIC"
> depends on MTK_PMIC_WRAP
--
~Randy
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] leds: mt6360: Add LED driver for MT6360
From: Pavel Machek @ 2020-06-04 13:52 UTC (permalink / raw)
To: Gene Chen
Cc: gene_chen, linux-kernel, cy_huang, benjamin.chao, linux-mediatek,
jacek.anaszewski, linux-leds, matthias.bgg, shufan_lee, Wilma.Wu,
linux-arm-kernel, dmurphy
In-Reply-To: <1591252004-12681-1-git-send-email-gene.chen.richtek@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1081 bytes --]
Hi!
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index c664d84..c47be91 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -229,6 +229,17 @@ config LEDS_MT6323
> This option enables support for on-chip LED drivers found on
> Mediatek MT6323 PMIC.
>
> +config LEDS_MT6360
> + tristate "LED Support for Mediatek MT6360 PMIC"
> + depends on LEDS_CLASS_FLASH && OF
> + depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> + depends on MFD_MT6360
> + help
> + This option enables support for dual Flash LED drivers found on
> + Mediatek MT6360 PMIC.
> + Support Torch and Strobe mode independently current source.
> + Include Low-VF and short protection.
This should be in english... and should not contain
"advertising". Just delete last two lines.
More useful information would be listing hardware where this is often
found.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] leds: mt6360: Add LED driver for MT6360
From: Pavel Machek @ 2020-06-04 13:50 UTC (permalink / raw)
To: Gene Chen
Cc: gene_chen, linux-kernel, cy_huang, benjamin.chao, linux-mediatek,
jacek.anaszewski, linux-leds, matthias.bgg, shufan_lee, Wilma.Wu,
linux-arm-kernel, dmurphy
In-Reply-To: <1591252004-12681-1-git-send-email-gene.chen.richtek@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 5708 bytes --]
Hi!
> +
> +#define MT6360_LED_DESC(_id) { \
> + .cdev = { \
> + .name = "mt6360_isink" #_id, \
> + .max_brightness = MT6360_SINKCUR_MAX##_id, \
> + .brightness_set_blocking = mt6360_led_brightness_set, \
> + .brightness_get = mt6360_led_brightness_get, \
> + .blink_set = mt6360_led_blink_set, \
> + }, \
> + .index = MT6360_LED_ISINK##_id, \
> + .currsel_reg = MT6360_CURRSEL_REG##_id, \
> + .currsel_mask = MT6360_CURRSEL_MASK##_id, \
> + .enable_mask = MT6360_LEDEN_MASK##_id, \
> + .mode_reg = MT6360_LEDMODE_REG##_id, \
> + .mode_mask = MT6360_LEDMODE_MASK##_id, \
> + .pwmduty_reg = MT6360_PWMDUTY_REG##_id, \
> + .pwmduty_mask = MT6360_PWMDUTY_MASK##_id, \
> + .pwmfreq_reg = MT6360_PWMFREQ_REG##_id, \
> + .pwmfreq_mask = MT6360_PWMFREQ_MASK##_id, \
> + .breath_regbase = MT6360_BREATH_REGBASE##_id, \
> +}
> +
> +/* ISINK 1/2/3 for RGBLED, ISINK4 for MoonLight */
> +static const struct mt6360_led_classdev def_led_classdev[MT6360_LED_MAX] = {
> + MT6360_LED_DESC(1),
> + MT6360_LED_DESC(2),
> + MT6360_LED_DESC(3),
> + MT6360_LED_DESC(4),
> +};
While this is pretty interesting abuse of the macros, please get rid
of it. I'm sure code will look better as a result.
> +static int mt6360_fled_strobe_set(
> + struct led_classdev_flash *fled_cdev, bool state)
> +{
> + struct led_classdev *led_cdev = &fled_cdev->led_cdev;
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)fled_cdev;
> + int id = mtfled_cdev->index, ret;
> +
> + if (!(state ^ test_bit(id, &mld->fl_strobe_flags)))
> + return 0;
Yup, and you can abuse xor operator too. Don't do that. I believe you
wanted to say:
+ if (state == test_bit(id, &mld->fl_strobe_flags))
+ return 0;
> + if (mt6360_fled_check_flags_if_any(&mld->fl_torch_flags)) {
> + dev_err(led_cdev->dev,
> + "Disable all leds torch [%lu]\n", mld->fl_torch_flags);
> + return -EINVAL;
> + }
> + ret = regmap_update_bits(mld->regmap, mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, state ? 0xff : 0);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set cs enable [%d]\n", state);
> + goto out_strobe_set;
> + }
Just return ret; no need for goto. (Please fix globally).
> +static int mt6360_fled_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_flash *lcf = lcdev_to_flcdev(led_cdev);
> + struct mt6360_led_data *mld = dev_get_drvdata(led_cdev->dev->parent);
> + struct mt6360_fled_classdev *mtfled_cdev = (void *)lcf;
> + int id = mtfled_cdev->index, shift, keep, ret;
> +
> + if (mt6360_fled_check_flags_if_any(&mld->fl_strobe_flags)) {
> + dev_err(led_cdev->dev,
> + "Disable all leds strobe [%lu]\n", mld->fl_strobe_flags);
> + return -EINVAL;
> + }
> + if (brightness == LED_OFF) {
> + clear_bit(id, &mld->fl_torch_flags);
> + keep = mt6360_fled_check_flags_if_any(&mld->fl_torch_flags);
> + ret = regmap_update_bits(mld->regmap,
> + mtfled_cdev->torch_enable_reg,
> + mtfled_cdev->torch_enable_mask,
> + keep ? 0xff : 0);
> + if (ret < 0) {
> + dev_err(led_cdev->dev, "Fail to set torch disable\n");
> + goto out_bright_set;
> + }
> + ret = regmap_update_bits(mld->regmap,
> + mtfled_cdev->cs_enable_reg,
> + mtfled_cdev->cs_enable_mask, 0);
> + if (ret < 0)
> + dev_err(led_cdev->dev, "Fail to set torch disable\n");
> + goto out_bright_set;
> + }
Should turning off the led go to separate function?
> +#define MT6360_FLED_DESC(_id) { \
> + .fl_cdev = { \
> + .led_cdev = { \
> + .name = "mt6360_fled_ch" #_id, \
> + .max_brightness = MT6360_TORBRIGHT_MAX##_id, \
> + .brightness_set_blocking = mt6360_fled_brightness_set, \
> + .brightness_get = mt6360_fled_brightness_get, \
> + .flags = LED_DEV_CAP_FLASH, \
> + }, \
> + .brightness = { \
> + .min = MT6360_STROBECUR_MIN, \
> + .step = MT6360_STROBECUR_STEP, \
> + .max = MT6360_STROBECUR_MAX, \
> + .val = MT6360_STROBECUR_MIN, \
> + }, \
> + .timeout = { \
> + .min = MT6360_STRBTIMEOUT_MIN, \
> + .step = MT6360_STRBTIMEOUT_STEP, \
> + .max = MT6360_STRBTIMEOUT_MAX, \
> + .val = MT6360_STRBTIMEOUT_MIN, \
> + }, \
> + .ops = &mt6360_fled_ops, \
> + }, \
> + .index = MT6360_FLED_CH##_id, \
> + .cs_enable_reg = MT6360_CSENABLE_REG##_id, \
> + .cs_enable_mask = MT6360_CSENABLE_MASK##_id, \
> + .torch_bright_reg = MT6360_TORBRIGHT_REG##_id, \
> + .torch_bright_mask = MT6360_TORBRIGHT_MASK##_id, \
> + .torch_enable_reg = MT6360_TORENABLE_REG##_id, \
> + .torch_enable_mask = MT6360_TORENABLE_MASK##_id, \
> + .strobe_bright_reg = MT6360_STRBRIGHT_REG##_id, \
> + .strobe_bright_mask = MT6360_STRBRIGHT_MASK##_id, \
> + .strobe_enable_reg = MT6360_STRBENABLE_REG##_id, \
> + .strobe_enable_mask = MT6360_STRBENABLE_MASK##_id, \
> +}
> +
> +static const struct mt6360_fled_classdev def_fled_classdev[MT6360_FLED_MAX] = {
> + MT6360_FLED_DESC(1),
> + MT6360_FLED_DESC(2),
> +};
Yeah, well, no.
> @@ -236,5 +241,4 @@ struct mt6360_pmu_data {
> #define CHIP_VEN_MASK (0xF0)
> #define CHIP_VEN_MT6360 (0x50)
> #define CHIP_REV_MASK (0x0F)
> -
> #endif /* __MT6360_H__ */
This one is unrelated and not really an improvement.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] regulator: mt6360: Add support for MT6360 regulator
From: Mark Brown @ 2020-06-04 13:39 UTC (permalink / raw)
To: Gene Chen
Cc: gene_chen, lgirdwood, linux-kernel, cy_huang, benjamin.chao,
linux-mediatek, matthias.bgg, Wilma.Wu, linux-arm-kernel,
shufan_lee
In-Reply-To: <1591254387-10304-1-git-send-email-gene.chen.richtek@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2567 bytes --]
On Thu, Jun 04, 2020 at 03:06:27PM +0800, Gene Chen wrote:
This looks nice and simple, a few fairly small comments below but high
level it's basically fine.
> --- /dev/null
> +++ b/drivers/regulator/mt6360-regulator.c
> @@ -0,0 +1,571 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
Please make the entire comment a C++ one so things look more
intentional.
> + for (i = 0; i < devdata->num_irq_descs; i++) {
> + irq_desc = devdata->irq_descs + i;
> + if (unlikely(!irq_desc->name))
> + continue;
Do we really need an unlikely here? This shouldn't be a hot path.
> +static int mt6360_regulator_set_mode(
> + struct regulator_dev *rdev, unsigned int mode)
> +{
> + switch (1 << (ffs(mode) - 1)) {
> + case REGULATOR_MODE_NORMAL:
I don't understand why this isn't just a straight switch on mode?
> +static unsigned int mt6360_regulator_get_mode(struct regulator_dev *rdev)
> +{
> + const struct mt6360_regulator_desc *desc =
> + (const struct mt6360_regulator_desc *)rdev->desc;
> + int shift = ffs(desc->mode_get_mask) - 1, ret;
> + unsigned int val = 0;
> +
> + default:
> + ret = 0;
> + }
If we can't parse a valid value from the hardware then that's an error.
> +static int mt6360_regulator_reg_write(void *context,
> + unsigned int reg, unsigned int val)
> +{
> + struct mt6360_regulator_data *mrd = context;
> + u8 chunk[4] = {0};
> +
> + /* chunk 0 ->i2c addr, 1 -> reg_addr, 2 -> reg_val 3-> crc8 */
> + chunk[0] = (mrd->i2c->addr & 0x7f) << 1;
> + chunk[1] = reg & 0x3f;
> + chunk[2] = (u8)val;
> + chunk[3] = crc8(mrd->crc8_table, chunk, 3, 0);
> + /* also dummy one byte */
> + return i2c_smbus_write_i2c_block_data(mrd->i2c, chunk[1], 3, chunk + 2);
> +}
Oh, wow - that's a fun I/O interface!
> +static const struct of_device_id __maybe_unused mt6360_regulator_of_id[] = {
> + {
> + .compatible = "mediatek,mt6360_pmic",
> + .data = (void *)&mt6360_pmic_devdata,
> + },
> + {
> + .compatible = "mediatek,mt6360_ldo",
> + .data = (void *)&mt6360_ldo_devdata,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mt6360_regulator_of_id);
I don't see any DT bindings documentation for this, documentation is
required for all new bindings.
> + mrd->regmap = devm_regmap_init(&(mrd->i2c->dev),
> + NULL, mrd, devdata->regmap_config);
> + if (IS_ERR(mrd->regmap)) {
> + dev_err(&pdev->dev, "Failed to register regmap\n");
> + return PTR_ERR(mrd->regmap);
> + }
This looks like a MFD so it's surprising to see us defining a regmap at
this level. Why are we doing this?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox