Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v8 2/4] dt-bindings: firmware: arm,scmi: support pinctrl protocol
From: Dhruva Gole @ 2024-04-08 10:14 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Linus Walleij, Dan Carpenter, Andy Shevchenko,
	linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
In-Reply-To: <20240405-pinctrl-scmi-v8-2-5fc8e33871bf@nxp.com>

On Apr 05, 2024 at 09:59:33 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add SCMI v3.2 pinctrl protocol bindings and example.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Reviewed-by: Dhruva Gole <d-gole@ti.com>

>  .../devicetree/bindings/firmware/arm,scmi.yaml     | 50 ++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4591523b51a0..e9d3f043c4ed 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -247,6 +247,37 @@ properties:
>        reg:
[...]

-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
From: AngeloGioacchino Del Regno @ 2024-04-08 10:16 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: chunkuang.hu, robh, krzysztof.kozlowski+dt, conor+dt, p.zabel,
	airlied, daniel, maarten.lankhorst, mripard, tzimmermann,
	matthias.bgg, shawn.sung, yu-chang.lee, ck.hu, jitao.shi,
	devicetree, linux-kernel, dri-devel, linux-mediatek,
	linux-arm-kernel, kernel
In-Reply-To: <CAGXv+5F9rfTVDExKSCF7fBKwR+HijNzFYE6+4aHKw3ZP81DG9w@mail.gmail.com>

Il 08/04/24 05:20, Chen-Yu Tsai ha scritto:
> On Thu, Apr 4, 2024 at 4:16 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
>> per HW instance (so potentially up to six displays for multi-vdo SoCs).
>>
>> The MMSYS or VDOSYS is always the first component in the DDP pipeline,
>> so it only supports an output port with multiple endpoints - where each
>> endpoint defines the starting point for one of the (currently three)
>> possible hardware paths.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   .../bindings/arm/mediatek/mediatek,mmsys.yaml | 23 +++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
>> index b3c6888c1457..90758bb5bcb1 100644
>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
>> @@ -93,6 +93,29 @@ properties:
>>     '#reset-cells':
>>       const: 1
>>
>> +  port:
>> +    $ref: /schemas/graph.yaml#/properties/port
>> +    description:
>> +      Output port node. This port connects the MMSYS/VDOSYS output to
>> +      the first component of one display pipeline, for example one of
>> +      the available OVL or RDMA blocks.
>> +      Some MediaTek SoCs support up to three display outputs per MMSYS.
>> +    properties:
>> +      endpoint@0:
>> +        $ref: /schemas/graph.yaml#/properties/endpoint
>> +        description: Output to the primary display pipeline
>> +
>> +      endpoint@1:
>> +        $ref: /schemas/graph.yaml#/properties/endpoint
>> +        description: Output to the secondary display pipeline
>> +
>> +      endpoint@2:
>> +        $ref: /schemas/graph.yaml#/properties/endpoint
>> +        description: Output to the tertiary display pipeline
>> +
>> +      required:
>> +        - endpoint@0
>> +
> 
> Technically the mmsys device serves as an glue layer for the display
> pipeline, providing things like clock control and signal routing; the
> device itself is not part of the pipeline, and probably shouldn't be
> part of the graph?
> 

That is (only) partially true: in the case of older SoCs, the MMSYS can only
connect to a single first IP of the pipeline, but in the case of newer ones,
and especially (but not limited to) MT8195 onwards having multiple instances
of VDOSYS, that really becomes part of the pipeline.

This is not because of the possible different first IP in the pipeline, but
because of support for dual-interface (DSI and DP) that, in even newer SoCs,
can be done with cross-mmsys (cross-vdosys, actually...) as some of those do
have the two in different VDOs.

So yes, this can be done without the graph in MMSYS *in this precise moment in
time*, but we'll anyway end up adding it sooner than later - and I'm doing this
right now, instead of later, because it's also simplifying the implementation
so like that I'm "catching two birds with one stone" :-)

Cheers,
Angelo

> ChenYu
> 
>>   required:
>>     - compatible
>>     - reg
>> --
>> 2.44.0
>>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v8 1/4] firmware: arm_scmi: introduce helper get_max_msg_size
From: Dhruva Gole @ 2024-04-08 10:22 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Sudeep Holla, Cristian Marussi, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Linus Walleij, Dan Carpenter, Andy Shevchenko,
	linux-arm-kernel, linux-kernel, devicetree, linux-gpio, Peng Fan
In-Reply-To: <20240405-pinctrl-scmi-v8-1-5fc8e33871bf@nxp.com>

On Apr 05, 2024 at 09:59:32 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> When Agent sending data to SCMI server, the Agent driver could check
> the size to avoid protocol buffer overflow. So introduce the helper
> get_max_msg_size.
> 
> Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/arm_scmi/driver.c    | 15 +++++++++++++++
>  drivers/firmware/arm_scmi/protocols.h |  2 ++
>  2 files changed, 17 insertions(+)

[...]

Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 2/2] mailbox: arm_mhuv3: Add driver
From: Cristian Marussi @ 2024-04-08 10:40 UTC (permalink / raw)
  To: Jassi Brar
  Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <CABb+yY3yzj167ypLPPV7OvqDWfv9y9EFQeZ5pxPAx7xUyyvLVw@mail.gmail.com>

On Sun, Apr 07, 2024 at 08:14:23PM -0500, Jassi Brar wrote:
> On Thu, Apr 4, 2024 at 1:25 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
> >
> > Add support for ARM MHUv3 mailbox controller.
> >
> > Support is limited to the MHUv3 Doorbell extension using only the PBX/MBX
> > combined interrupts.
> >

Hi Jassi,

thanks for having a look at this !

> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > v1 -> v2
> > - fixed checkpatch warnings about side-effects
> > - fixed sparse errors as reported
> >   | Reported-by: kernel test robot <lkp@intel.com>
> >   | Closes: https://lore.kernel.org/oe-kbuild-all/202403290015.tCLXudqC-lkp@intel.com/
> > ---
> >  MAINTAINERS                 |    9 +
> >  drivers/mailbox/Kconfig     |   11 +
> >  drivers/mailbox/Makefile    |    2 +
> >  drivers/mailbox/arm_mhuv3.c | 1063 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 1085 insertions(+)
> >  create mode 100644 drivers/mailbox/arm_mhuv3.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index aa3b947fb080..e957b9d9e32a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12998,6 +12998,15 @@ F:     Documentation/devicetree/bindings/mailbox/arm,mhuv2.yaml
> >  F:     drivers/mailbox/arm_mhuv2.c
> >  F:     include/linux/mailbox/arm_mhuv2_message.h
> >
> > +MAILBOX ARM MHUv3
> > +M:     Sudeep Holla <sudeep.holla@arm.com>
> > +M:     Cristian Marussi <cristian.marussi@arm.com>
> > +L:     linux-kernel@vger.kernel.org
> > +L:     linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> > +S:     Maintained
> > +F:     Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> > +F:     drivers/mailbox/arm_mhuv3.c
> > +
> >  MAN-PAGES: MANUAL PAGES FOR LINUX -- Sections 2, 3, 4, 5, and 7
> >  M:     Alejandro Colomar <alx@kernel.org>
> >  L:     linux-man@vger.kernel.org
> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> > index 42940108a187..d20cdae65cfe 100644
> > --- a/drivers/mailbox/Kconfig
> > +++ b/drivers/mailbox/Kconfig
> > @@ -23,6 +23,17 @@ config ARM_MHU_V2
> >           Say Y here if you want to build the ARM MHUv2 controller driver,
> >           which provides unidirectional mailboxes between processing elements.
> >
> > +config ARM_MHU_V3
> > +       tristate "ARM MHUv3 Mailbox"
> > +       depends on ARM64 || COMPILE_TEST
> > +       help
> > +         Say Y here if you want to build the ARM MHUv3 controller driver,
> > +         which provides unidirectional mailboxes between processing elements.
> > +
> > +         ARM MHUv3 controllers can implement a varying number of extensions
> > +         that provides different means of transports: supported extensions
> > +         will be discovered and possibly managed at probe-time.
> > +
> >  config IMX_MBOX
> >         tristate "i.MX Mailbox"
> >         depends on ARCH_MXC || COMPILE_TEST
> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> > index 18793e6caa2f..5cf2f54debaf 100644
> > --- a/drivers/mailbox/Makefile
> > +++ b/drivers/mailbox/Makefile
> > @@ -9,6 +9,8 @@ obj-$(CONFIG_ARM_MHU)   += arm_mhu.o arm_mhu_db.o
> >
> >  obj-$(CONFIG_ARM_MHU_V2)       += arm_mhuv2.o
> >
> > +obj-$(CONFIG_ARM_MHU_V3)       += arm_mhuv3.o
> > +
> >  obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o
> >
> >  obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX)    += armada-37xx-rwtm-mailbox.o
> > diff --git a/drivers/mailbox/arm_mhuv3.c b/drivers/mailbox/arm_mhuv3.c
> > new file mode 100644
> > index 000000000000..e4125568bec0
> > --- /dev/null
> > +++ b/drivers/mailbox/arm_mhuv3.c
> > @@ -0,0 +1,1063 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ARM Message Handling Unit Version 3 (MHUv3) driver.
> > + *
> > + * Copyright (C) 2024 ARM Ltd.
> > + *
> > + * Based on ARM MHUv2 driver.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mailbox_controller.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +
> > +/* ====== MHUv3 Registers ====== */
> > +
> > +/* Maximum number of Doorbell channel windows */
> > +#define MHUV3_DBCW_MAX                 128
> > +/* Number of DBCH combined interrupt status registers */
> > +#define MHUV3_DBCH_CMB_INT_ST_REG_CNT  4
> > +#define MHUV3_INVALID_DOORBELL         0xFFFFFFFFUL
> > +
> > +/* Number of FFCH combined interrupt status registers */
> > +#define MHUV3_FFCH_CMB_INT_ST_REG_CNT  2
> > +
> > +#define MHUV3_STAT_BYTES               (sizeof(u32))
> >
> Simply 4 please.
>

Ok.
 
> > +#define MHUV3_STAT_BITS                        (MHUV3_STAT_BYTES * __CHAR_BIT__)
> >
> just 32.
>

Ok.
 
> > +
> > +/* Not a typo ... */
> > +#define MHUV3_MAJOR_VERSION            2
> > +
> > +enum {
> > +       MHUV3_MBOX_CELL_TYPE,
> > +       MHUV3_MBOX_CELL_CHWN,
> > +       MHUV3_MBOX_CELL_PARAM,
> > +       MHUV3_MBOX_CELLS
> > +};
> > +
> > +/* CTRL_Page */
> > +
> > +struct blk_id {
> > +       u32 blk_id : 4;
> 
> Please avoid name clashes.
> 

I'll fix.

> > +       u32 pad : 28;
> > +} __packed;
> > +
> > +struct feat_spt0 {
> > +       u32 dbe_spt : 4;
> > +       u32 fe_spt : 4;
> > +       u32 fce_spt : 4;
> > +       u32 tze_spt : 4;
> > +       u32 rme_spt : 4;
> > +       u32 rase_spt : 4;
> > +       u32 pad: 8;
> > +} __packed;
> > +
> > +struct feat_spt1 {
> > +       u32 auto_op_spt : 4;
> > +       u32 pad: 28;
> > +} __packed;
> > +
> > +struct dbch_cfg0 {
> > +       u32 num_dbch : 8;
> > +       u32 pad: 24;
> > +} __packed;
> > +
> > +struct ffch_cfg0 {
> > +       u32 num_ffch : 8;
> > +       u32 x8ba_spt : 1;
> > +       u32 x16ba_spt : 1;
> > +       u32 x32ba_spt : 1;
> > +       u32 x64ba_spt : 1;
> > +       u32 pad : 4;
> > +       u32 ffch_depth : 10;
> > +       u32 pad2 : 6;
> > +} __packed;
> > +
> > +struct fch_cfg0 {
> > +       u32 num_fch : 10;
> > +       /* MBX only registers */
> > +       u32 fcgi_spt : 1;
> > +       /* ------------------ */
> > +       u32 num_fcg : 5;
> > +       u32 num_fch_per_grp : 5;
> > +       u32 fch_ws : 8;
> > +       u32 pad : 3;
> > +} __packed;
> > +
> > +struct ctrl {
> > +       u32 op_req : 1;
> > +       u32 ch_op_mask : 1;
> > +       u32 pad : 30;
> > +} __packed;
> > +
> > +struct fch_ctrl {
> > +       u32 pad : 2;
> > +       u32 int_en : 1;
> > +       u32 pad2 : 29;
> > +} __packed;
> > +
> > +struct iidr {
> > +       u32 implementer : 12;
> > +       u32 revision : 4;
> > +       u32 variant : 4;
> > +       u32 product_id : 12;
> > +} __packed;
> > +
> > +struct aidr {
> > +       u32 arch_minor_rev : 4;
> > +       u32 arch_major_rev : 4;
> > +       u32 pad : 24;
> > +} __packed;
> > +
> I am not sure about using bitfields on register values. I know v2
> driver also uses bitfields but this still is not very portable and is
> dependent on compiler behaviour. We may actually save some loc by not
> having unused fields if we use shifts and masks. Though I don't
> strongly feel either way.
> 

Yes, indeed seemed a bit odd way of handling regs when I saw it in mhuv2,
BUT it seemed it had its advantages in terms of clarity of usage....did
not know about possible drawbacks, though. I'll re-think about the pros
and cons of this approach.

> > +struct ctrl_page {
> > +       struct blk_id blk_id;
> > +       u8 pad[0x10 - 0x4];
> > +       struct feat_spt0 feat_spt0;
> > +       struct feat_spt1 feat_spt1;
> > +       u8 pad1[0x20 - 0x18];
> > +       struct dbch_cfg0 dbch_cfg0;
> > +       u8 pad2[0x30 - 0x24];
> > +       struct ffch_cfg0 ffch_cfg0;
> > +       u8 pad3[0x40 - 0x34];
> > +       struct fch_cfg0 fch_cfg0;
> > +       u8 pad4[0x100 - 0x44];
> > +       struct ctrl ctrl;
> > +       /* MBX only registers */
> > +       u8 pad5[0x140 - 0x104];
> > +       struct fch_ctrl fch_ctrl;
> > +       u32 fcg_int_en;
> > +       u8 pad6[0x400 - 0x148];
> > +       /* ------------------ */
> Why the decoration ? Maybe comment on what different starts from here.
> 

PBX and MBX Ctrl page are exactly the same, BUT for some registers banks
that does not exist in the PBX: this decoration is indeed the end, not
the start, of the MBX only regs that starts 5 lines above with the related
comment...was trying to avoid to use 2 different types for the basically
the same data...of course it works just because the PBX code refrains
from accessing the areas where only regs known to MBX lives.

> > +       u32 dbch_int_st[MHUV3_DBCH_CMB_INT_ST_REG_CNT];
> > +       u32 ffch_int_st[MHUV3_FFCH_CMB_INT_ST_REG_CNT];
> > +       /* MBX only registers */
> > +       u8 pad7[0x470 - 0x418];
> > +       u32 fcg_int_st;
> > +       u8 pad8[0x480 - 0x474];
> > +       u32 fcg_grp_int_st[32];
> > +       u8 pad9[0xFC8 - 0x500];
> > +       /* ------------------ */

Same here.

> > +       struct iidr iidr;
> > +       struct aidr aidr;
> > +       u32 imp_def_id[12];
> > +} __packed;
> > +
> > +/* DBCW_Page */
> > +
> > +struct xbcw_ctrl {
> > +       u32 comb_en : 1;
> > +       u32 pad : 31;
> > +} __packed;
> > +
> > +struct pdbcw_int {
> > +       u32 tfr_ack : 1;
> > +       u32 pad : 31;
> > +} __packed;
> > +
> > +struct pdbcw_page {
> > +       u32 st;
> > +       u8 pad[0xC - 0x4];
> > +       u32 set;
> > +       struct pdbcw_int int_st;
> > +       struct pdbcw_int int_clr;
> > +       struct pdbcw_int int_en;
> > +       struct xbcw_ctrl ctrl;
> > +} __packed;
> > +
> > +struct mdbcw_page {
> > +       u32 st;
> > +       u32 st_msk;
> > +       u32 clr;
> > +       u8 pad[0x10 - 0xC];
> > +       u32 msk_st;
> > +       u32 msk_set;
> > +       u32 msk_clr;
> > +       struct xbcw_ctrl ctrl;
> > +} __packed;
> > +
> > +struct dummy_page {
> > +       u8 pad[0x1000];
> > +} __packed;
> > +
> > +struct mhu3_pbx_frame_reg {
> > +       struct ctrl_page ctrl;
> > +       struct pdbcw_page dbcw[MHUV3_DBCW_MAX];
> > +       struct dummy_page ffcw;
> > +       struct dummy_page fcw;
> > +       u8 pad[0xF000 - 0x4000];
> > +       struct dummy_page impdef;
> > +} __packed;
> > +
> > +struct mhu3_mbx_frame_reg {
> > +       struct ctrl_page ctrl;
> > +       struct mdbcw_page dbcw[MHUV3_DBCW_MAX];
> > +       struct dummy_page ffcw;
> > +       struct dummy_page fcw;
> > +       u8 pad[0xF000 - 0x4000];
> > +       struct dummy_page impdef;
> > +} __packed;
> > +
> > +/* Macro for reading a bitfield within a physically mapped packed struct */
> > +#define readl_relaxed_bitfield(_regptr, _field)                                \
> > +       ({                                                              \
> > +               u32 _rval;                                              \
> > +               typeof(_regptr) _rptr = _regptr;                        \
> > +               _rval = readl_relaxed(_rptr);                           \
> > +               ((typeof(*_rptr) __force *)(&_rval))->_field;           \
> > +       })
> > +
> > +/* Macro for writing a bitfield within a physically mapped packed struct */
> > +#define writel_relaxed_bitfield(_value, _regptr, _field)               \
> > +       ({                                                              \
> > +               u32 _rval;                                              \
> > +               typeof(_regptr) _rptr = _regptr;                        \
> > +               _rval = readl_relaxed(_rptr);                           \
> > +               ((typeof(*_rptr) __force *)(&_rval))->_field = _value;  \
> > +               writel_relaxed(_rval, _rptr);                           \
> > +       })
> > +
> > +/* ====== MHUv3 data structures ====== */
> > +
> > +enum mhuv3_frame {
> > +       PBX_FRAME,
> > +       MBX_FRAME
> > +};
> > +
> > +static char *mhuv3_str[] = {
> > +       "PBX",
> > +       "MBX"
> > +};
> > +
> > +enum mhuv3_extension_type {
> > +       FIRST_EXT = 0,
> > +       DBE_EXT = FIRST_EXT,
> > +       FCE_EXT,
> > +       FE_EXT,
> > +       MAX_EXT
> > +};
> > +
> > +struct mhuv3;
> > +
> > +/**
> > + * struct mhuv3_protocol_ops - MHUv3 operations
> > + *
> > + * @rx_startup: Receiver startup callback.
> > + * @rx_shutdown: Receiver shutdown callback.
> > + * @read_data: Read available Sender in-band LE data (if any).
> > + * @rx_complete: Acknowledge data reception to the Sender. Any out-of-band data
> > + *              has to have been already retrieved before calling this.
> > + * @tx_startup: Sender startup callback.
> > + * @tx_shutdown: Sender shutdown callback.
> > + * @last_tx_done: Report back to the Sender if the last transfer has completed.
> > + * @send_data: Send data to the receiver.
> > + *
> > + * Each supported transport protocol provides its own implementation of
> > + * these operations.
> > + */
> > +struct mhuv3_protocol_ops {
> > +       int (*rx_startup)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       void (*rx_shutdown)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       void *(*read_data)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       void (*rx_complete)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       void (*tx_startup)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       void (*tx_shutdown)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       int (*last_tx_done)(struct mhuv3 *mhu, struct mbox_chan *chan);
> > +       int (*send_data)(struct mhuv3 *mhu, struct mbox_chan *chan, void *arg);
> > +};
> > +
> > +/**
> > + * struct mhuv3_mbox_chan_priv - MHUv3 channel private information
> > + *
> > + * @ch_idx: Channel window index associated to this mailbox channel.
> > + * @doorbell: Doorbell bit number within the @ch_idx window.
> > + *           Only relevant to Doorbell transport.
> > + * @ops: Transport protocol specific operations for this channel.
> > + *
> > + * Transport specific data attached to mmailbox channel priv data.
> > + */
> > +struct mhuv3_mbox_chan_priv {
> > +       u32 ch_idx;
> > +       u32 doorbell;
> > +       const struct mhuv3_protocol_ops *ops;
> > +};
> > +
> > +/**
> > + * struct mhuv3_extension - MHUv3 extension descriptor
> > + *
> > + * @type: Type of extension
> > + * @max_chans: Max number of channels found for this extension.
> > + * @base_ch_idx: First channel number assigned to this extension, picked from
> > + *              the set of all mailbox channels descriptors created.
> > + * @mbox_of_xlate: Extension specific helper to parse DT and lookup associated
> > + *                channel from the related 'mboxes' property.
> > + * @combined_irq_setup: Extension specific helper to setup the combined irq.
> > + * @channels_init: Extension specific helper to initialize channels.
> > + * @chan_from_comb_irq_get: Extension specific helper to lookup which channel
> > + *                         triggered the combined irq.
> > + * @pending_db: Array of per-channel pending doorbells.
> > + * @pending_lock: Protect access to pending_db.
> > + */
> > +struct mhuv3_extension {
> > +       enum mhuv3_extension_type type;
> > +       unsigned int max_chans;
> > +       unsigned int base_ch_idx;
> > +       struct mbox_chan *(*mbox_of_xlate)(struct mhuv3 *mhu,
> > +                                          unsigned int channel,
> > +                                          unsigned int param);
> > +       void (*combined_irq_setup)(struct mhuv3 *mhu);
> > +       int (*channels_init)(struct mhuv3 *mhu);
> > +       struct mbox_chan *(*chan_from_comb_irq_get)(struct mhuv3 *mhu);
> > +       u32 pending_db[MHUV3_DBCW_MAX];
> > +       /* Protect access to pending_db */
> > +       spinlock_t pending_lock;
> > +};
> > +
> > +/**
> > + * struct mhuv3 - MHUv3 mailbox controller data
> > + *
> > + * @frame:     Frame type: MBX_FRAME or PBX_FRAME.
> > + * @auto_op_full: Flag to indicate if the MHU supports AutoOp full mode.
> > + * @major: MHUv3 controller architectural major version.
> > + * @minor: MHUv3 controller architectural minor version.
> > + * @tot_chans: The total number of channnels discovered across all extensions.
> > + * @cmb_irq: Combined IRQ number if any found defined.
> > + * @ctrl: A reference to the MHUv3 control page for this block.
> > + * @pbx: Base address of the PBX register mapping region.
> > + * @mbx: Base address of the MBX register mapping region.
> > + * @ext: Array holding descriptors for any found implemented extension.
> > + * @mbox: Mailbox controller belonging to the MHU frame.
> > + */
> > +struct mhuv3 {
> > +       enum mhuv3_frame frame;
> > +       bool auto_op_full;
> > +       unsigned int major;
> > +       unsigned int minor;
> > +       unsigned int tot_chans;
> >
> may be num_chans or chan_count ?
> 

Ok.

> 
> > +       int cmb_irq;
> > +       struct ctrl_page __iomem *ctrl;
> > +       union {
> > +               struct mhu3_pbx_frame_reg __iomem *pbx;
> > +               struct mhu3_mbx_frame_reg __iomem *mbx;
> > +       };
> > +       struct mhuv3_extension *ext[MAX_EXT];
> > +       struct mbox_controller mbox;
> > +};
> > +
> > +#define mhu_from_mbox(_mbox) container_of(_mbox, struct mhuv3, mbox)
> > +
> > +typedef int (*mhuv3_extension_initializer)(struct mhuv3 *mhu);
> > +
> > +/* =================== Doorbell transport protocol operations =============== */
> > +
> > +static void mhuv3_doorbell_tx_startup(struct mhuv3 *mhu, struct mbox_chan *chan)
> > +{
> > +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > +
> > +       /* Enable Transfer Acknowledgment events */
> > +       writel_relaxed_bitfield(0x1, &mhu->pbx->dbcw[priv->ch_idx].int_en, tfr_ack);
> > +}
> > +
> > +static void mhuv3_doorbell_tx_shutdown(struct mhuv3 *mhu, struct mbox_chan *chan)
> > +{
> > +       unsigned long flags;
> > +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> > +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > +
> In order of decreasing line-lengths please everywhere.
>

Sure.
 
> > +       /* Disable Channel Transfer Ack events */
> > +       writel_relaxed_bitfield(0x0, &mhu->pbx->dbcw[priv->ch_idx].int_en, tfr_ack);
> > +
> > +       /* Clear Channel Transfer Ack and pending doorbells */
> > +       writel_relaxed_bitfield(0x1, &mhu->pbx->dbcw[priv->ch_idx].int_clr, tfr_ack);
> > +       spin_lock_irqsave(&e->pending_lock, flags);
> > +       e->pending_db[priv->ch_idx] = 0;
> > +       spin_unlock_irqrestore(&e->pending_lock, flags);
> > +}

[snip]

> > +static struct mbox_chan *mhuv3_dbe_chan_from_comb_irq_get(struct mhuv3 *mhu)
> > +{
> > +       int i;
> > +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> > +       struct device *dev = mhu->mbox.dev;
> > +
> > +       for (i = 0; i < MHUV3_DBCH_CMB_INT_ST_REG_CNT; i++) {
> > +               unsigned int channel, db = MHUV3_INVALID_DOORBELL;
> > +               u32 cmb_st, st;
> > +
> > +               cmb_st = readl_relaxed(&mhu->ctrl->dbch_int_st[i]);
> > +               if (!cmb_st)
> > +                       continue;
> > +
> > +               channel = i * MHUV3_STAT_BITS + __builtin_ctz(cmb_st);
> 
> __ffs instead of __builtin_ctz please.
>

ok.
 
> > +               if (channel >= e->max_chans) {
> > +                       dev_err(dev, "Invalid %s channel:%d\n",
> > +                               mhuv3_str[mhu->frame], channel);
> > +                       break;
> > +               }
> > +

[snip]

> > +static irqreturn_t mhuv3_pbx_comb_interrupt(int irq, void *arg)
> > +{
> > +       int ret = IRQ_NONE;
> > +       unsigned int i, found = 0;
> > +       struct mhuv3 *mhu = arg;
> > +       struct device *dev = mhu->mbox.dev;
> > +       struct mbox_chan *chan;
> > +
> > +       for (i = FIRST_EXT; i < MAX_EXT; i++) {
> > +               /* FCE does not participate to the PBX combined */
> > +               if (i == FCE_EXT || !mhu->ext[i])
> > +                       continue;
> > +
> > +               chan = mhu->ext[i]->chan_from_comb_irq_get(mhu);
> > +               if (!IS_ERR(chan)) {
> >
>   'continue' for error instead, to have fewer indented lines.
>

ok.
 
> > +                       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> > +
> > +                       found++;
> > +                       if (chan->cl) {
> > +                               mbox_chan_txdone(chan, 0);
> > +                               ret = IRQ_HANDLED;
> > +                       } else {
> > +                               dev_warn(dev,
> > +                                        "TX Ack on UNBOUND channel (%u)\n",
> > +                                        priv->ch_idx);
> > +                       }
> > +               }
> > +       }
> > +
> > +       if (!found)
> > +               dev_warn_once(dev, "Failed to find channel for the TX interrupt\n");
> > +
> > +       return ret;
> > +}
> > +
> > +static irqreturn_t mhuv3_mbx_comb_interrupt(int irq, void *arg)
> > +{
> > +       int ret = IRQ_NONE;
> > +       unsigned int i, found = 0;
> > +       struct mhuv3 *mhu = arg;
> > +       struct device *dev = mhu->mbox.dev;
> > +       struct mbox_chan *chan;
> > +
> > +       for (i = FIRST_EXT; i < MAX_EXT; i++) {
> > +               if (!mhu->ext[i])
> > +                       continue;
> > +
> > +               /* Process any extension which could be source of the IRQ */
> > +               chan = mhu->ext[i]->chan_from_comb_irq_get(mhu);
> > +               if (!IS_ERR(chan)) {
>   'continue' for error instead, to have fewer indented lines.
>

ok.

Thanks,
Cristian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] dt-bindings: pinctrl: mediatek: mt7622: add "gpio-ranges" property
From: Rafał Miłecki @ 2024-04-08 10:51 UTC (permalink / raw)
  To: Sean Wang, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
	linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
	Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

Allow specifying pin to GPIO mapping. It can be find in in-Linux DTS
file for MT7622.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml   | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
index eaa65512161a..d211f2e403f2 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
@@ -36,6 +36,9 @@ properties:
       the amount of cells must be specified as 2. See the below mentioned gpio
       binding representation for description of particular cells.
 
+  gpio-ranges:
+    maxItems: 1
+
   interrupt-controller: true
 
   interrupts:
-- 
2.35.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] soc: xilinx: rename cpu_number1 to dummy_cpu_number
From: Jay Buddhabhatti @ 2024-04-08 11:06 UTC (permalink / raw)
  To: michal.simek; +Cc: linux-arm-kernel, linux-kernel, Jay Buddhabhatti

The per cpu variable cpu_number1 is passed to xlnx_event_handler as
argument "dev_id", but it is not used in this function. So drop the
initialization of this variable and rename it to dummy_cpu_number.
This patch is to fix the following call trace when the kernel option
CONFIG_DEBUG_ATOMIC_SLEEP is enabled:

BUG: sleeping function called from invalid context at include/linux/sched/mm.h:274
    in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1, name: swapper/0
    preempt_count: 1, expected: 0
    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0 #53
    Hardware name: Xilinx Versal vmk180 Eval board rev1.1 (QSPI) (DT)
    Call trace:
     dump_backtrace+0xd0/0xe0
     show_stack+0x18/0x40
     dump_stack_lvl+0x7c/0xa0
     dump_stack+0x18/0x34
     __might_resched+0x10c/0x140
     __might_sleep+0x4c/0xa0
     __kmem_cache_alloc_node+0xf4/0x168
     kmalloc_trace+0x28/0x38
     __request_percpu_irq+0x74/0x138
     xlnx_event_manager_probe+0xf8/0x298
     platform_probe+0x68/0xd8

Fixes: daed80ed0758 ("soc: xilinx: Fix for call trace due to the usage of smp_processor_id()")
Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@amd.com>
---
 drivers/soc/xilinx/xlnx_event_manager.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c
index 253299e4214d..366018f6a0ee 100644
--- a/drivers/soc/xilinx/xlnx_event_manager.c
+++ b/drivers/soc/xilinx/xlnx_event_manager.c
@@ -3,6 +3,7 @@
  * Xilinx Event Management Driver
  *
  *  Copyright (C) 2021 Xilinx, Inc.
+ *  Copyright (C) 2024 Advanced Micro Devices, Inc.
  *
  *  Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
  */
@@ -19,7 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-static DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number1);
+static DEFINE_PER_CPU_READ_MOSTLY(int, dummy_cpu_number);
 
 static int virq_sgi;
 static int event_manager_availability = -EACCES;
@@ -570,7 +571,6 @@ static void xlnx_disable_percpu_irq(void *data)
 static int xlnx_event_init_sgi(struct platform_device *pdev)
 {
 	int ret = 0;
-	int cpu;
 	/*
 	 * IRQ related structures are used for the following:
 	 * for each SGI interrupt ensure its mapped by GIC IRQ domain
@@ -607,11 +607,8 @@ static int xlnx_event_init_sgi(struct platform_device *pdev)
 	sgi_fwspec.param[0] = sgi_num;
 	virq_sgi = irq_create_fwspec_mapping(&sgi_fwspec);
 
-	cpu = get_cpu();
-	per_cpu(cpu_number1, cpu) = cpu;
 	ret = request_percpu_irq(virq_sgi, xlnx_event_handler, "xlnx_event_mgmt",
-				 &cpu_number1);
-	put_cpu();
+				 &dummy_cpu_number);
 
 	WARN_ON(ret);
 	if (ret) {
@@ -627,16 +624,12 @@ static int xlnx_event_init_sgi(struct platform_device *pdev)
 
 static void xlnx_event_cleanup_sgi(struct platform_device *pdev)
 {
-	int cpu = smp_processor_id();
-
-	per_cpu(cpu_number1, cpu) = cpu;
-
 	cpuhp_remove_state(CPUHP_AP_ONLINE_DYN);
 
 	on_each_cpu(xlnx_disable_percpu_irq, NULL, 1);
 
 	irq_clear_status_flags(virq_sgi, IRQ_PER_CPU);
-	free_percpu_irq(virq_sgi, &cpu_number1);
+	free_percpu_irq(virq_sgi, &dummy_cpu_number);
 	irq_dispose_mapping(virq_sgi);
 }
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v3 1/2] dt-bindings: mailbox: arm,mhuv3: Add bindings
From: Cristian Marussi @ 2024-04-08 11:09 UTC (permalink / raw)
  To: Jassi Brar
  Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <CABb+yY07OQQ8gAOZM2iV7nh_P5sQPWwNPODfwnJNZoPk8MNg2w@mail.gmail.com>

On Sun, Apr 07, 2024 at 06:38:52PM -0500, Jassi Brar wrote:
> On Thu, Apr 4, 2024 at 1:25 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
> >
> > Add bindings for the ARM MHUv3 Mailbox controller.
> >

Hi,

> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > v2 -> v3
> > - fixed spurious tabs in dt_binding_check
> > v1 -> v2
> > - clarified extension descriptions around configurability and discoverability
> > - removed unused labels from the example
> > - using pattern properties to define interrupt-names
> > - bumped interrupt maxItems to 74 (allowing uo to 8 channels per extension)
> > ---
> >  .../bindings/mailbox/arm,mhuv3.yaml           | 217 ++++++++++++++++++
> >  1 file changed, 217 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> > new file mode 100644
> > index 000000000000..32a8bb711464
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> > @@ -0,0 +1,217 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mailbox/arm,mhuv3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM MHUv3 Mailbox Controller
> > +
> > +maintainers:
> > +  - Sudeep Holla <sudeep.holla@arm.com>
> > +  - Cristian Marussi <cristian.marussi@arm.com>
> > +
> > +description: |
> > +  The Arm Message Handling Unit (MHU) Version 3 is a mailbox controller that
> > +  enables unidirectional communications with remote processors through various
> > +  possible transport protocols.
> > +  The controller can optionally support a varying number of extensions that, in
> > +  turn, enable different kinds of transport to be used for communication.
> > +  Number, type and characteristics of each supported extension can be discovered
> > +  dynamically at runtime.
> > +
> > +  Given the unidirectional nature of the controller, an MHUv3 mailbox controller
> > +  is composed of a MHU Sender (MHUS) containing a PostBox (PBX) block and a MHU
> > +  Receiver (MHUR) containing a MailBox (MBX) block, where
> > +
> > +   PBX is used to
> > +      - Configure the MHU
> > +      - Send Transfers to the Receiver
> > +      - Optionally receive acknowledgment of a Transfer from the Receiver
> > +
> > +   MBX is used to
> > +      - Configure the MHU
> > +      - Receive Transfers from the Sender
> > +      - Optionally acknowledge Transfers sent by the Sender
> > +
> > +  Both PBX and MBX need to be present and defined in the DT description if you
> > +  need to establish a bidirectional communication, since you will have to
> > +  acquire two distinct unidirectional channels, one for each block.
> > +
> > +  As a consequence both blocks needs to be represented separately and specified
> > +  as distinct DT nodes in order to properly describe their resources.
> > +
> > +  Note that, though, thanks to the runtime discoverability, there is no need to
> > +  identify the type of blocks with distinct compatibles.
> > +
> > +  Following are the MHUv3 possible extensions.
> > +
> > +  - Doorbell Extension (DBE): DBE defines a type of channel called a Doorbell
> > +    Channel (DBCH). DBCH enables a single bit Transfer to be sent from the
> > +    Sender to Receiver. The Transfer indicates that an event has occurred.
> > +    When DBE is implemented, the number of DBCHs that an implementation of the
> > +    MHU can support is between 1 and 128, numbered starting from 0 in ascending
> > +    order and discoverable at run-time.
> > +    Each DBCH contains 32 individual fields, referred to as flags, each of which
> > +    can be used independently. It is possible for the Sender to send multiple
> > +    Transfers at once using a single DBCH, so long as each Transfer uses
> > +    a different flag in the DBCH.
> > +    Optionally, data may be transmitted through an out-of-band shared memory
> > +    region, wherein the MHU Doorbell is used strictly as an interrupt generation
> > +    mechanism, but this is out of the scope of these bindings.
> > +
> > +  - FastChannel Extension (FCE): FCE defines a type of channel called a Fast
> > +    Channel (FCH). FCH is intended for lower overhead communication between
> > +    Sender and Receiver at the expense of determinism. An FCH allows the Sender
> > +    to update the channel value at any time, regardless of whether the previous
> > +    value has been seen by the Receiver. When the Receiver reads the channel's
> > +    content it gets the last value written to the channel.
> > +    FCH is considered lossy in nature, and means that the Sender has no way of
> > +    knowing if, or when, the Receiver will act on the Transfer.
> > +    FCHs are expected to behave as RAM which generates interrupts when writes
> > +    occur to the locations within the RAM.
> > +    When FCE is implemented, the number of FCHs that an implementation of the
> > +    MHU can support is between 1-1024, if the FastChannel word-size is 32-bits,
> > +    or between 1-512, when the FastChannel word-size is 64-bits.
> > +    FCHs are numbered from 0 in ascending order.
> > +    Note that the number of FCHs and the word-size are implementation defined,
> > +    not configurable but discoverable at run-time.
> > +    Optionally, data may be transmitted through an out-of-band shared memory
> > +    region, wherein the MHU FastChannel is used as an interrupt generation
> > +    mechanism which carries also a pointer to such out-of-band data, but this
> > +    is out of the scope of these bindings.
> > +
> > +  - FIFO Extension (FE): FE defines a Channel type called a FIFO Channel (FFCH).
> > +    FFCH allows a Sender to send
> > +       - Multiple Transfers to the Receiver without having to wait for the
> > +         previous Transfer to be acknowledged by the Receiver, as long as the
> > +         FIFO has room for the Transfer.
> > +       - Transfers which require the Receiver to provide acknowledgment.
> > +       - Transfers which have in-band payload.
> > +    In all cases, the data is guaranteed to be observed by the Receiver in the
> > +    same order which the Sender sent it.
> > +    When FE is implemented, the number of FFCHs that an implementation of the
> > +    MHU can support is between 1 and 64, numbered starting from 0 in ascending
> > +    order. The number of FFCHs, their depth (same for all implemented FFCHs) and
> > +    the access-granularity are implementation defined, not configurable but
> > +    discoverable at run-time.
> > +    Optionally, additional data may be transmitted through an out-of-band shared
> > +    memory region, wherein the MHU FIFO is used to transmit, in order, a small
> > +    part of the payload (like a header) and a reference to the shared memory
> > +    area holding the remaining, bigger, chunk of the payload, but this is out of
> > +    the scope of these bindings.
> > +
> > +properties:
> > +  compatible:
> > +    const: arm,mhuv3
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 74
> > +
> > +  interrupt-names:
> > +    description: |
> > +      The MHUv3 controller generates a number of events some of which are used
> > +      to generate interrupts; as a consequence it can expose a varying number of
> > +      optional PBX/MBX interrupts, representing the events generated during the
> > +      operation of the various transport protocols associated with different
> > +      extensions. All interrupts of the MHU are level-sensitive.
> > +      Some of these optional interrupts are defined per-channel, where the
> > +      number of channels effectively available is implementation defined and
> > +      run-time discoverable.
> > +      In the following names are enumerated using patterns, with per-channel
> > +      interrupts implicitly capped at the maximum channels allowed by the
> > +      specification for each extension type.
> > +      For the sake of simplicity maxItems is anyway capped to a most plausible
> > +      number, assuming way less channels would be implemented than actually
> > +      possible.
> > +
> > +      The only mandatory interrupts on the MHU are:
> > +        - combined
> > +        - mbx-fch-xfer-<N> but only if mbx-fcgrp-xfer-<N> is not implemented.
> > +
> > +    minItems: 1
> > +    maxItems: 74
> > +    items:
> > +      oneOf:
> > +        - const: combined
> > +          description: PBX/MBX Combined interrupt
> > +        - const: combined-ffch
> > +          description: PBX/MBX FIFO Combined interrupt
> > +        - pattern: '^ffch-low-tide-[0-9]+$'
> > +          description: PBX/MBX FIFO Channel <N> Low Tide interrupt
> > +        - pattern: '^ffch-high-tide-[0-9]+$'
> > +          description: PBX/MBX FIFO Channel <N> High Tide interrupt
> > +        - pattern: '^ffch-flush-[0-9]+$'
> > +          description: PBX/MBX FIFO Channel <N> Flush interrupt
> > +        - pattern: '^mbx-dbch-xfer-[0-9]+$'
> > +          description: MBX Doorbell Channel <N> Transfer interrupt
> > +        - pattern: '^mbx-fch-xfer-[0-9]+$'
> > +          description: MBX FastChannel <N> Transfer interrupt
> > +        - pattern: '^mbx-fchgrp-xfer-[0-9]+$'
> > +          description: MBX FastChannel <N> Group Transfer interrupt
> > +        - pattern: '^mbx-ffch-xfer-[0-9]+$'
> > +          description: MBX FIFO Channel <N> Transfer interrupt
> > +        - pattern: '^pbx-dbch-xfer-ack-[0-9]+$'
> > +          description: PBX Doorbell Channel <N> Transfer Ack interrupt
> > +        - pattern: '^pbx-ffch-xfer-ack-[0-9]+$'
> > +          description: PBX FIFO Channel <N> Transfer Ack interrupt
> > +
> Can we have optional subnodes (with different properties as required)
> for each extension type ?
>

Not sure if I have understood properly you request, but the type of extensions
present in a PBX/MBX block can be discovered at runtime together with their
characteristics (like the number of channels) so the specific DT properties
can be searched (or NOT) based on the features discovered at run-time:
are you asking for subnodes as a means of adding clarity to what can be
defined in a block depending on what it is (PBX vs MBX) and what
extensions it has ?

if that is the reason ... the convoluted names like pbx/mbx-dbch- etc...
was my attempt at thhat :D ... to give a hint at what you can define in a PBX
vs MBX block and what is related to each extensions...

Note that, though, since the PBX/MBX blocks are discoverable at runtime as such,
they are not identified as such in the DT (same compatible) so I would not have
anywayy the capability to check in the DT which is which (PBX/MBX) and if the
properties are appropriate or not, nor I could know which extensions are really
implemented, so such subnodes would ony be a way of grouping props in the DT
without adding any compile time check capability nor adding any improvement to
the runtime DT parsing proces...

...BUT, of course, I could be wrong and missing a something here, so I
am happy to corrected on the subnodes utility...

> 
> > +  '#mbox-cells':
> > +    description: |
> > +      The first argument in the consumers 'mboxes' property represents the
> > +      extension type, the second is for the channel number while the third
> > +      depends on extension type.
> > +
> > +      Extension type for DBE is 0 and the third parameter represents the
> > +      doorbell flag number to use.
> > +      Extension type for FCE is 1, third parameter unused.
> > +      Extension type for FE is 2, third parameter unused.
> > +
> > +      mboxes = <&mhu 0 0 5>; // DBE, Doorbell Channel Window 0, doorbell flag 5.
> > +      mboxes = <&mhu 0 1 7>; // DBE, Doorbell Channel Window 1, doorbell flag 7.
> > +      mboxes = <&mhu 1 0 0>; // FCE, FastChannel Window 0.
> > +      mboxes = <&mhu 1 3 0>; // FCE, FastChannel Window 3.
> > +      mboxes = <&mhu 2 1 0>; // FE, FIFO Channel Window 1.
> > +      mboxes = <&mhu 2 7 0>; // FE, FIFO Channel Window 7.
> >
> Please define the extension types, instead of 0, 1 and 2.
> 

I'll do.

Thanks for the review.
Cristian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 1/2] media: dt-bindings: nxp,imx8-jpeg:: Add clocks entries
From: Fabio Estevam @ 2024-04-08 11:14 UTC (permalink / raw)
  To: Mirela Rabulea
  Cc: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam,
	alexander.stein, Frank.li, ming.qian, conor+dt, devicetree,
	linux-arm-kernel, s.hauer, kernel, mchehab, hverkuil, linux-media,
	imx, linux-kernel
In-Reply-To: <20240408075806.1194297-1-mirela.rabulea@nxp.com>

Hi Mirela,

On Mon, Apr 8, 2024 at 4:58 AM Mirela Rabulea <mirela.rabulea@nxp.com> wrote:
>
> From: Fabio Estevam <festevam@gmail.com>
>
> The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
> the PER and IPG clocks to be functional, so add the clock entries.
>
> This also fixes the following schema warning:
>
> imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>

Thanks for submitting this series.

In the Subject, you added an extra : "..nxp,imx8-jpeg:: Add". Please
remove the extra :.

My From and Signed-off-by tags e-mails do not match.

Please use the email from the Signed-off-by in the From field as well.

The same applies to patch 2/2.

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dt-bindings: pinctrl: mediatek: mt7622: add "gpio-ranges" property
From: Krzysztof Kozlowski @ 2024-04-08 11:25 UTC (permalink / raw)
  To: Rafał Miłecki, Sean Wang, Linus Walleij, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
	linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
	Rafał Miłecki
In-Reply-To: <20240408105128.30586-1-zajec5@gmail.com>

On 08/04/2024 12:51, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Allow specifying pin to GPIO mapping. It can be find in in-Linux DTS
> file for MT7622.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>


Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Fabio Estevam @ 2024-04-08 11:31 UTC (permalink / raw)
  To: Ian Ray
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20240408092449.6-1-ian.ray@gehealthcare.com>

Hi Ian,

> +       pinctrl_i2c1_gpio: i2c1gpiogrp {
> +               fsl,pins =
> +                       <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14              0x400001c3>,

The Sion bit is unnecessary in the GPIO mode so you could pass 0x1c3 instead.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: mailmap: update Richard Genoud's email address
From: Nicolas Ferre @ 2024-04-08 11:32 UTC (permalink / raw)
  To: Richard Genoud, Greg Kroah-Hartman
  Cc: Alexandre Belloni, linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20240408101329.9448-1-richard.genoud@bootlin.com>

On 08/04/2024 at 12:13, Richard Genoud wrote:
> I'm working now at bootlin, so I'll use my bootlin address for kernel
> development from now on.
> 
> Update also the yaml file for atmel-serial accordingly.
> 
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>

Thanks for the update Richard!
Reviewed-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
   Nicolas

> ---
>   .mailmap                                                       | 1 +
>   Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml | 2 +-
>   MAINTAINERS                                                    | 2 +-
>   3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 8284692f9610..71e28f4e0d4a 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -524,6 +524,7 @@ Rémi Denis-Courmont <rdenis@simphalempin.com>
>   Ricardo Ribalda <ribalda@kernel.org> <ricardo@ribalda.com>
>   Ricardo Ribalda <ribalda@kernel.org> Ricardo Ribalda Delgado <ribalda@kernel.org>
>   Ricardo Ribalda <ribalda@kernel.org> <ricardo.ribalda@gmail.com>
> +Richard Genoud <richard.genoud@bootlin.com> <richard.genoud@gmail.com>
>   Richard Leitner <richard.leitner@linux.dev> <dev@g0hl1n.net>
>   Richard Leitner <richard.leitner@linux.dev> <me@g0hl1n.net>
>   Richard Leitner <richard.leitner@linux.dev> <richard.leitner@skidata.com>
> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> index 65cb2e5c5eee..eb2992a447d7 100644
> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> @@ -8,7 +8,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
>   title: Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
> 
>   maintainers:
> -  - Richard Genoud <richard.genoud@gmail.com>
> +  - Richard Genoud <richard.genoud@bootlin.com>
> 
>   properties:
>     compatible:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aea47e04c3a5..0dbdc81e46c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14363,7 +14363,7 @@ F:      drivers/dma/at_xdmac.c
>   F:     include/dt-bindings/dma/at91.h
> 
>   MICROCHIP AT91 SERIAL DRIVER
> -M:     Richard Genoud <richard.genoud@gmail.com>
> +M:     Richard Genoud <richard.genoud@bootlin.com>
>   S:     Maintained
>   F:     Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>   F:     drivers/tty/serial/atmel_serial.c


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Ian Ray @ 2024-04-08 11:44 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <CAOMZO5B-vTRSfi=tNc_iZxnxYstL8JJOd_1rMf4ps9WHyfx0GQ@mail.gmail.com>

On Mon, Apr 08, 2024 at 08:31:53AM -0300, Fabio Estevam wrote:
> 
> Hi Ian,
> 
> > +       pinctrl_i2c1_gpio: i2c1gpiogrp {
> > +               fsl,pins =
> > +                       <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14              0x400001c3>,
> 
> The Sion bit is unnecessary in the GPIO mode so you could pass 0x1c3 instead.

Thank you -- I will submit a V2.

Would 0x1c2 be more correct?  From the IMX8MPRM.pdf, it seems that the
lowest bit is reserved.  Example: 8.2.4.158 SW_PAD_CTL_PAD_GPIO1_IO05
SW PAD Control Register (IOMUXC_SW_PAD_CTL_PAD_GPIO1_IO05).

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/4] KVM: delete .change_pte MMU notifier callback
From: Michael Ellerman @ 2024-04-08 11:45 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm
  Cc: Marc Zyngier, Oliver Upton, Tianrui Zhao, Bibo Mao,
	Thomas Bogendoerfer, Nicholas Piggin, Anup Patel, Atish Patra,
	Sean Christopherson, Andrew Morton, David Hildenbrand,
	linux-arm-kernel, kvmarm, loongarch, linux-mips, linuxppc-dev,
	kvm-riscv, linux-mm, linux-trace-kernel, linux-perf-users
In-Reply-To: <20240405115815.3226315-2-pbonzini@redhat.com>

Paolo Bonzini <pbonzini@redhat.com> writes:
> The .change_pte() MMU notifier callback was intended as an
> optimization. The original point of it was that KSM could tell KVM to flip
> its secondary PTE to a new location without having to first zap it. At
> the time there was also an .invalidate_page() callback; both of them were
> *not* bracketed by calls to mmu_notifier_invalidate_range_{start,end}(),
> and .invalidate_page() also doubled as a fallback implementation of
> .change_pte().
>
> Later on, however, both callbacks were changed to occur within an
> invalidate_range_start/end() block.
>
> In the case of .change_pte(), commit 6bdb913f0a70 ("mm: wrap calls to
> set_pte_at_notify with invalidate_range_start and invalidate_range_end",
> 2012-10-09) did so to remove the fallback from .invalidate_page() to
> .change_pte() and allow sleepable .invalidate_page() hooks.
>
> This however made KVM's usage of the .change_pte() callback completely
> moot, because KVM unmaps the sPTEs during .invalidate_range_start()
> and therefore .change_pte() has no hope of finding a sPTE to change.
> Drop the generic KVM code that dispatches to kvm_set_spte_gfn(), as
> well as all the architecture specific implementations.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/arm64/kvm/mmu.c                  | 34 -----------------
>  arch/loongarch/include/asm/kvm_host.h |  1 -
>  arch/loongarch/kvm/mmu.c              | 32 ----------------
>  arch/mips/kvm/mmu.c                   | 30 ---------------
>  arch/powerpc/include/asm/kvm_ppc.h    |  1 -
>  arch/powerpc/kvm/book3s.c             |  5 ---
>  arch/powerpc/kvm/book3s.h             |  1 -
>  arch/powerpc/kvm/book3s_64_mmu_hv.c   | 12 ------
>  arch/powerpc/kvm/book3s_hv.c          |  1 -
>  arch/powerpc/kvm/book3s_pr.c          |  7 ----
>  arch/powerpc/kvm/e500_mmu_host.c      |  6 ---

LGTM.

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Fabio Estevam @ 2024-04-08 11:48 UTC (permalink / raw)
  To: Ian Ray
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
	linux-arm-kernel, linux-kernel
In-Reply-To: <ZhPYkGLM_b5IEKs2@de2cfed78370>

On Mon, Apr 8, 2024 at 8:44 AM Ian Ray <ian.ray@gehealthcare.com> wrote:

> Thank you -- I will submit a V2.
>
> Would 0x1c2 be more correct?  From the IMX8MPRM.pdf, it seems that the
> lowest bit is reserved.  Example: 8.2.4.158 SW_PAD_CTL_PAD_GPIO1_IO05
> SW PAD Control Register (IOMUXC_SW_PAD_CTL_PAD_GPIO1_IO05).

Yes, good catch. It is better not to write to the reserved bit 0, so
0x1c2 is preferred.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 2/2] media: i2c: Add GC05A2 image sensor driver
From: Zhi Mao (毛智) @ 2024-04-08 11:50 UTC (permalink / raw)
  To: mchehab@kernel.org, sakari.ailus@linux.intel.com,
	robh+dt@kernel.org, kieran.bingham@ideasonboard.com,
	krzysztof.kozlowski+dt@linaro.org
  Cc: heiko@sntech.de, gerald.loacker@wolfvision.net,
	linux-kernel@vger.kernel.org, yunkec@chromium.org,
	linux-mediatek@lists.infradead.org, dan.scally@ideasonboard.com,
	linux-media@vger.kernel.org,
	Shengnan Wang (王圣男), hdegoede@redhat.com,
	linus.walleij@linaro.org, andy.shevchenko@gmail.com,
	Yaya Chang (張雅清), bingbu.cao@intel.com,
	jacopo.mondi@ideasonboard.com, jernej.skrabec@gmail.com,
	devicetree@vger.kernel.org, conor+dt@kernel.org,
	Project_Global_Chrome_Upstream_Group, 10572168@qq.com,
	hverkuil-cisco@xs4all.nl, tomi.valkeinen@ideasonboard.com,
	linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
	laurent.pinchart@ideasonboard.com,
	angelogioacchino.delregno@collabora.com, macromorgan@hotmail.com
In-Reply-To: <171248091995.2374960.12981271990757968652@ping.linuxembedded.co.uk>

Hi Kieran,

Thanks for your review this patch.

It seems that there are some difficult for us(Mediatek) to explain
these register setting comments.
As these settings are released by GC sensor vendor, and we have not
detailed datasheet described them.
And even if send the letter to ask sensor vendor, I am afraid there may
be not a clear response.

Can we just focus on the driver code function and control flow part?




On Sun, 2024-04-07 at 10:08 +0100, Kieran Bingham wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hello,
> 
> Thanks for helping extending the kernels sensor driver support.
> 
> My comments below can likely be taken with a pinch of salt, as they
> are
> mostly around the tabled register values ... but we have many drivers
> which are binary blobs of sensor register values and I think it would
> be
> far more beneficial to clean these up where possible...
> 
> So the first question is ... Can we ?
> 
> 
> 
> Quoting Zhi Mao (2024-04-03 04:38:25)
> > Add a V4L2 sub-device driver for Galaxycore GC05A2 image sensor.
> > 
> > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> > ---
> >  drivers/media/i2c/Kconfig  |   10 +
> >  drivers/media/i2c/Makefile |    1 +
> >  drivers/media/i2c/gc05a2.c | 1383
> ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 1394 insertions(+)
> >  create mode 100644 drivers/media/i2c/gc05a2.c
> > 
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 56f276b920ab..97993bf160f9 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -70,6 +70,16 @@ config VIDEO_GC0308
> >           To compile this driver as a module, choose M here: the
> >           module will be called gc0308.
> >  
> > +config VIDEO_GC05A2
> > +       tristate "GalaxyCore gc05a2 sensor support"
> > +       select V4L2_CCI_I2C
> > +       help
> > +         This is a Video4Linux2 sensor driver for the GalaxyCore
> gc05a2
> > +         camera.
> > +
> > +         To compile this driver as a module, choose M here: the
> > +         module will be called gc05a2.
> > +
> >  config VIDEO_GC2145
> >         select V4L2_CCI_I2C
> >         tristate "GalaxyCore GC2145 sensor support"
> > diff --git a/drivers/media/i2c/Makefile
> b/drivers/media/i2c/Makefile
> > index dfbe6448b549..8ed6faf0f854 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> >  obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> >  obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> >  obj-$(CONFIG_VIDEO_GC0308) += gc0308.o
> > +obj-$(CONFIG_VIDEO_GC05A2) += gc05a2.o
> >  obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> >  obj-$(CONFIG_VIDEO_HI556) += hi556.o
> >  obj-$(CONFIG_VIDEO_HI846) += hi846.o
> > diff --git a/drivers/media/i2c/gc05a2.c
> b/drivers/media/i2c/gc05a2.c
> > new file mode 100644
> > index 000000000000..461d33055a3b
> > --- /dev/null
> > +++ b/drivers/media/i2c/gc05a2.c
> > @@ -0,0 +1,1383 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for GalaxyCore gc05a2 image sensor
> > + *
> > + * Copyright 2024 MediaTek
> > + *
> > + * Zhi Mao <zhi.mao@mediatek.com>
> > + */
> > +#include <linux/array_size.h>
> > +#include <linux/bits.h>
> > +#include <linux/clk.h>
> > +#include <linux/container_of.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/math64.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/types.h>
> > +#include <linux/units.h>
> > +
> > +#include <media/v4l2-cci.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define GC05A2_REG_TEST_PATTERN_EN CCI_REG8(0x008c)
> > +#define GC05A2_REG_TEST_PATTERN_IDX CCI_REG8(0x008d)
> > +#define GC05A2_TEST_PATTERN_EN 0x01
> > +
> > +#define GC05A2_STREAMING_REG CCI_REG8(0x0100)
> > +
> > +#define GC05A2_FLIP_REG CCI_REG8(0x0101)
> > +#define GC05A2_FLIP_H_MASK BIT(0)
> > +#define GC05A2_FLIP_V_MASK BIT(1)
> > +
> > +#define GC05A2_EXP_REG CCI_REG16(0x0202)
> > +#define GC05A2_EXP_MARGIN 16
> > +#define GC05A2_EXP_MIN 4
> > +#define GC05A2_EXP_STEP 1
> > +
> > +#define GC05A2_AGAIN_REG CCI_REG16(0x0204)
> > +#define GC05A2_AGAIN_MIN 1024
> > +#define GC05A2_AGAIN_MAX (1024 * 16)
> > +#define GC05A2_AGAIN_STEP 1
> > +
> > +#define GC05A2_FRAME_LENGTH_REG CCI_REG16(0x0340)
> > +#define GC05A2_VTS_MAX 0xffff
> > +
> > +#define GC05A2_REG_CHIP_ID CCI_REG16(0x03f0)
> > +#define GC05A2_CHIP_ID 0x05a2
> > +
> > +#define GC05A2_NATIVE_WIDTH 2592
> > +#define GC05A2_NATIVE_HEIGHT 1944
> > +
> > +#define GC05A2_DEFAULT_CLK_FREQ (24 * HZ_PER_MHZ)
> > +#define GC05A2_MBUS_CODE MEDIA_BUS_FMT_SGRBG10_1X10
> > +#define GC05A2_DATA_LANES 2
> > +#define GC05A2_RGB_DEPTH 10
> > +#define GC05A2_SLEEP_US  (2 * USEC_PER_MSEC)
> > +
> > +static const char *const gc05a2_test_pattern_menu[] = {
> > +       "No Pattern",  "Fade_to_gray_Color Bar", "Color Bar",
> > +       "PN9",         "Horizental_gradient",    "Checkboard
> Pattern",
> > +       "Slant",       "Resolution",             "Solid Black",
> > +       "Solid White",
> > +};
> > +
> > +static const s64 gc05a2_link_freq_menu_items[] = {
> > +       (448 * HZ_PER_MHZ),
> > +       (224 * HZ_PER_MHZ),
> > +};
> > +
> > +static const char *const gc05a2_supply_name[] = {
> > +       "avdd",
> > +       "dvdd",
> > +       "dovdd",
> > +};
> > +
> > +struct gc05a2 {
> > +       struct device *dev;
> > +       struct v4l2_subdev sd;
> > +       struct media_pad pad;
> > +
> > +       struct clk *xclk;
> > +       struct regulator_bulk_data
> supplies[ARRAY_SIZE(gc05a2_supply_name)];
> > +       struct gpio_desc *reset_gpio;
> > +
> > +       struct v4l2_ctrl_handler ctrls;
> > +       struct v4l2_ctrl *pixel_rate;
> > +       struct v4l2_ctrl *link_freq;
> > +       struct v4l2_ctrl *exposure;
> > +       struct v4l2_ctrl *vblank;
> > +       struct v4l2_ctrl *hblank;
> > +       struct v4l2_ctrl *hflip;
> > +       struct v4l2_ctrl *vflip;
> > +
> > +       struct regmap *regmap;
> > +       unsigned long link_freq_bitmap;
> > +
> > +       /* True if the device has been identified */
> > +       bool identified;
> > +       const struct gc05a2_mode *cur_mode;
> > +};
> > +
> > +struct gc05a2_reg_list {
> > +       u32 num_of_regs;
> > +       const struct cci_reg_sequence *regs;
> > +};
> > +
> > +static const struct cci_reg_sequence mode_2592x1944[] = {
> > +       /* system */
> > +       { CCI_REG8(0x0135), 0x01 },
> > +
> > +       /* pre_setting */
> > +       { CCI_REG8(0x0084), 0x21 },
> > +       { CCI_REG8(0x0d05), 0xcc },
> > +       { CCI_REG8(0x0218), 0x00 },
> > +       { CCI_REG8(0x005e), 0x48 },
> > +       { CCI_REG8(0x0d06), 0x01 },
> > +       { CCI_REG8(0x0007), 0x16 },
> > +       { CCI_REG8(0x0101), 0x00 },
> > +
> > +       /* analog */
> > +       { CCI_REG8(0x0342), 0x07 },
> > +       { CCI_REG8(0x0343), 0x28 },
> > +       { CCI_REG8(0x0220), 0x07 },
> > +       { CCI_REG8(0x0221), 0xd0 },
> > +       { CCI_REG8(0x0202), 0x07 },
> > +       { CCI_REG8(0x0203), 0x32 },
> > +       { CCI_REG8(0x0340), 0x07 },
> > +       { CCI_REG8(0x0341), 0xf0 },
> > +       { CCI_REG8(0x0219), 0x00 },
> > +       { CCI_REG8(0x0346), 0x00 },
> > +       { CCI_REG8(0x0347), 0x04 },
> > +       { CCI_REG8(0x0d14), 0x00 },
> > +       { CCI_REG8(0x0d13), 0x05 },
> > +       { CCI_REG8(0x0d16), 0x05 },
> > +       { CCI_REG8(0x0d15), 0x1d },
> > +       { CCI_REG8(0x00c0), 0x0a },
> > +       { CCI_REG8(0x00c1), 0x30 },
> > +       { CCI_REG8(0x034a), 0x07 },
> > +       { CCI_REG8(0x034b), 0xa8 },
> > +       { CCI_REG8(0x0e0a), 0x00 },
> > +       { CCI_REG8(0x0e0b), 0x00 },
> > +       { CCI_REG8(0x0e0e), 0x03 },
> > +       { CCI_REG8(0x0e0f), 0x00 },
> > +       { CCI_REG8(0x0e06), 0x0a },
> > +       { CCI_REG8(0x0e23), 0x15 },
> > +       { CCI_REG8(0x0e24), 0x15 },
> > +       { CCI_REG8(0x0e2a), 0x10 },
> > +       { CCI_REG8(0x0e2b), 0x10 },
> > +       { CCI_REG8(0x0e17), 0x49 },
> > +       { CCI_REG8(0x0e1b), 0x1c },
> > +       { CCI_REG8(0x0e3a), 0x36 },
> > +       { CCI_REG8(0x0d11), 0x84 },
> > +       { CCI_REG8(0x0e52), 0x14 },
> > +       { CCI_REG8(0x000b), 0x10 },
> > +       { CCI_REG8(0x0008), 0x08 },
> > +       { CCI_REG8(0x0223), 0x17 },
> > +       { CCI_REG8(0x0d27), 0x39 },
> > +       { CCI_REG8(0x0d22), 0x00 },
> > +       { CCI_REG8(0x03f6), 0x0d },
> > +       { CCI_REG8(0x0d04), 0x07 },
> > +       { CCI_REG8(0x03f3), 0x72 },
> > +       { CCI_REG8(0x03f4), 0xb8 },
> > +       { CCI_REG8(0x03f5), 0xbc },
> > +       { CCI_REG8(0x0d02), 0x73 },
> > +
> > +       /* auto load start */
> > +       { CCI_REG8(0x00cb), 0x00 },
> > +
> > +       /* OUT 2592*1944 */
> > +       { CCI_REG8(0x0350), 0x01 },
> > +       { CCI_REG8(0x0353), 0x00 },
> > +       { CCI_REG8(0x0354), 0x08 },
> 
> > +       { CCI_REG8(0x034c), 0x0a },
> > +       { CCI_REG8(0x034d), 0x20 },
> 
> Should/Could this be
>         { CCI_REG16(0x034c), 2592 }, /* Width */
> 
> 
> > +       { CCI_REG8(0x021f), 0x14 },
> > +
> > +       /* MIPI */
> > +       { CCI_REG8(0x0107), 0x05 },
> > +       { CCI_REG8(0x0117), 0x01 },
> > +       { CCI_REG8(0x0d81), 0x00 },
> > +       { CCI_REG8(0x0d84), 0x0c },
> > +       { CCI_REG8(0x0d85), 0xa8 },
> > +       { CCI_REG8(0x0d86), 0x06 },
> > +       { CCI_REG8(0x0d87), 0x55 },
> > +       { CCI_REG8(0x0db3), 0x06 },
> > +       { CCI_REG8(0x0db4), 0x08 },
> > +       { CCI_REG8(0x0db5), 0x1e },
> > +       { CCI_REG8(0x0db6), 0x02 },
> > +       { CCI_REG8(0x0db8), 0x12 },
> > +       { CCI_REG8(0x0db9), 0x0a },
> > +       { CCI_REG8(0x0d93), 0x06 },
> > +       { CCI_REG8(0x0d94), 0x09 },
> > +       { CCI_REG8(0x0d95), 0x0d },
> > +       { CCI_REG8(0x0d99), 0x0b },
> > +       { CCI_REG8(0x0084), 0x01 },
> > +
> > +       /* OUT */
> > +       { CCI_REG8(0x0110), 0x01 },
> > +};
> > +
> > +static const struct cci_reg_sequence mode_1280x720[] = {
> > +       /* system */
> > +       { CCI_REG8(0x0135), 0x05 },
> 
> In 2592x1944 this is 0x01. Do you have a datasheet? Can you explain
> why
> they are different? Can you add register definitions that have names
> to
> make this more maintainable or extendable in the future?
> 
> There's discussion in the recent series improving the IMX258 which
> makes
> me wonder if we should try harder to have sensor drivers with clearer
> definitions.
> 
> 
> > +
> > +       /*pre_setting*/
> 
> /* pre_setting */ ?
> 
> > +       { CCI_REG8(0x0084), 0x21 },
> > +       { CCI_REG8(0x0d05), 0xcc },
> > +       { CCI_REG8(0x0218), 0x80 },
> > +       { CCI_REG8(0x005e), 0x49 },
> > +       { CCI_REG8(0x0d06), 0x81 },
> > +       { CCI_REG8(0x0007), 0x16 },
> > +       { CCI_REG8(0x0101), 0x00 },
> 
> In 2592x1944, only register 0x0218 differs. Why? What is that? Can it
> be
> broken out to a function that applies the correct configuration at
> startuup based on a parameter instead of duplicating this table set?
> 
> > +
> > +       /* analog */
> > +       { CCI_REG8(0x0342), 0x07 },
> > +       { CCI_REG8(0x0343), 0x10 },
> > +       { CCI_REG8(0x0220), 0x07 },
> > +       { CCI_REG8(0x0221), 0xd0 },
> > +       { CCI_REG8(0x0202), 0x03 },
> > +       { CCI_REG8(0x0203), 0x32 },
> > +       { CCI_REG8(0x0340), 0x04 },
> > +       { CCI_REG8(0x0341), 0x08 },
> > +       { CCI_REG8(0x0219), 0x00 },
> > +       { CCI_REG8(0x0346), 0x01 },
> > +       { CCI_REG8(0x0347), 0x00 },
> > +       { CCI_REG8(0x0d14), 0x00 },
> > +       { CCI_REG8(0x0d13), 0x05 },
> > +       { CCI_REG8(0x0d16), 0x05 },
> > +       { CCI_REG8(0x0d15), 0x1d },
> > +       { CCI_REG8(0x00c0), 0x0a },
> > +       { CCI_REG8(0x00c1), 0x30 },
> > +       { CCI_REG8(0x034a), 0x05 },
> > +       { CCI_REG8(0x034b), 0xb0 },
> > +       { CCI_REG8(0x0e0a), 0x00 },
> > +       { CCI_REG8(0x0e0b), 0x00 },
> > +       { CCI_REG8(0x0e0e), 0x03 },
> > +       { CCI_REG8(0x0e0f), 0x00 },
> > +       { CCI_REG8(0x0e06), 0x0a },
> > +       { CCI_REG8(0x0e23), 0x15 },
> > +       { CCI_REG8(0x0e24), 0x15 },
> > +       { CCI_REG8(0x0e2a), 0x10 },
> > +       { CCI_REG8(0x0e2b), 0x10 },
> > +       { CCI_REG8(0x0e17), 0x49 },
> > +       { CCI_REG8(0x0e1b), 0x1c },
> > +       { CCI_REG8(0x0e3a), 0x36 },
> > +       { CCI_REG8(0x0d11), 0x84 },
> > +       { CCI_REG8(0x0e52), 0x14 },
> > +       { CCI_REG8(0x000b), 0x0e },
> > +       { CCI_REG8(0x0008), 0x03 },
> > +       { CCI_REG8(0x0223), 0x16 },
> > +       { CCI_REG8(0x0d27), 0x39 },
> > +       { CCI_REG8(0x0d22), 0x00 },
> > +       { CCI_REG8(0x03f6), 0x0d },
> > +       { CCI_REG8(0x0d04), 0x07 },
> > +       { CCI_REG8(0x03f3), 0x72 },
> > +       { CCI_REG8(0x03f4), 0xb8 },
> > +       { CCI_REG8(0x03f5), 0xbc },
> > +       { CCI_REG8(0x0d02), 0x73 },
> > +
> 
> Are any of those able to be broken out to named register to be more
> clear in their intent?
> 
> > +       /* auto load start */
> > +       { CCI_REG8(0x00cb), 0xfc },
> > +
> 
> Why is this auto load start so different to the other modes 'auto
> load
> start'? What do the bits refer to ?
> 
> > +       /* OUT 1280x720 */
> > +       { CCI_REG8(0x0350), 0x01 },
> > +       { CCI_REG8(0x0353), 0x00 },
> > +       { CCI_REG8(0x0354), 0x0c },
> 
> > +       { CCI_REG8(0x034c), 0x05 },
> > +       { CCI_REG8(0x034d), 0x00 },
> 
> Should/Could this be 
>          { CCI_REG16(0x034c), 1280 },
> 
> Are there any other register settings that would make more sense to
> be
> in decimal units that match their actual context?
> 
> 
> > +       { CCI_REG8(0x021f), 0x14 },
> 
> I don't see a setting for 720/0x2d0. Do these registers only set the
> width?
> 
> > +
> > +       /* MIPI */
> > +       { CCI_REG8(0x0107), 0x05 },
> > +       { CCI_REG8(0x0117), 0x01 },
> > +       { CCI_REG8(0x0d81), 0x00 },
> > +       { CCI_REG8(0x0d84), 0x06 },
> > +       { CCI_REG8(0x0d85), 0x40 },
> > +       { CCI_REG8(0x0d86), 0x03 },
> > +       { CCI_REG8(0x0d87), 0x21 },
> > +       { CCI_REG8(0x0db3), 0x03 },
> > +       { CCI_REG8(0x0db4), 0x04 },
> > +       { CCI_REG8(0x0db5), 0x0d },
> > +       { CCI_REG8(0x0db6), 0x01 },
> > +       { CCI_REG8(0x0db8), 0x04 },
> > +       { CCI_REG8(0x0db9), 0x06 },
> > +       { CCI_REG8(0x0d93), 0x03 },
> > +       { CCI_REG8(0x0d94), 0x04 },
> > +       { CCI_REG8(0x0d95), 0x05 },
> > +       { CCI_REG8(0x0d99), 0x06 },
> > +       { CCI_REG8(0x0084), 0x01 },
> > +
> > +       /* OUT */
> 
> Out where? What is out?
> 
> > +       { CCI_REG8(0x0110), 0x01 },
> > +};
> > +
> > +static const struct cci_reg_sequence mode_table_common[] = {
> > +       { GC05A2_STREAMING_REG, 0x00 },
> > +       /* system */
> > +       { CCI_REG8(0x0315), 0xd4 },
> > +       { CCI_REG8(0x0d06), 0x01 },
> > +       { CCI_REG8(0x0a70), 0x80 },
> > +       { CCI_REG8(0x031a), 0x00 },
> > +       { CCI_REG8(0x0314), 0x00 },
> > +       { CCI_REG8(0x0130), 0x08 },
> > +       { CCI_REG8(0x0132), 0x01 },
> > +       { CCI_REG8(0x0136), 0x38 },
> > +       { CCI_REG8(0x0137), 0x03 },
> > +       { CCI_REG8(0x0134), 0x5b },
> > +       { CCI_REG8(0x031c), 0xe0 },
> > +       { CCI_REG8(0x0d82), 0x14 },
> > +       { CCI_REG8(0x0dd1), 0x56 },
> > +
> > +       /* gate_mode */
> > +       { CCI_REG8(0x0af4), 0x01 },
> > +       { CCI_REG8(0x0002), 0x10 },
> > +       { CCI_REG8(0x00c3), 0x34 },
> > +
> > +       /* auto load start */
> 
> The previous 'auto load start' referenced 0x00cb ?
> 
> > +       { CCI_REG8(0x00c4), 0x00 },
> > +       { CCI_REG8(0x00c5), 0x01 },
> > +       { CCI_REG8(0x0af6), 0x00 },
> > +       { CCI_REG8(0x0ba0), 0x17 },
> > +       { CCI_REG8(0x0ba1), 0x00 },
> > +       { CCI_REG8(0x0ba2), 0x00 },
> > +       { CCI_REG8(0x0ba3), 0x00 },
> > +       { CCI_REG8(0x0ba4), 0x03 },
> > +       { CCI_REG8(0x0ba5), 0x00 },
> > +       { CCI_REG8(0x0ba6), 0x00 },
> > +       { CCI_REG8(0x0ba7), 0x00 },
> > +       { CCI_REG8(0x0ba8), 0x40 },
> > +       { CCI_REG8(0x0ba9), 0x00 },
> > +       { CCI_REG8(0x0baa), 0x00 },
> > +       { CCI_REG8(0x0bab), 0x00 },
> > +       { CCI_REG8(0x0bac), 0x40 },
> > +       { CCI_REG8(0x0bad), 0x00 },
> > +       { CCI_REG8(0x0bae), 0x00 },
> > +       { CCI_REG8(0x0baf), 0x00 },
> > +       { CCI_REG8(0x0bb0), 0x02 },
> > +       { CCI_REG8(0x0bb1), 0x00 },
> > +       { CCI_REG8(0x0bb2), 0x00 },
> > +       { CCI_REG8(0x0bb3), 0x00 },
> > +       { CCI_REG8(0x0bb8), 0x02 },
> > +       { CCI_REG8(0x0bb9), 0x00 },
> > +       { CCI_REG8(0x0bba), 0x00 },
> > +       { CCI_REG8(0x0bbb), 0x00 },
> > +       { CCI_REG8(0x0a70), 0x80 },
> > +       { CCI_REG8(0x0a71), 0x00 },
> > +       { CCI_REG8(0x0a72), 0x00 },
> > +       { CCI_REG8(0x0a66), 0x00 },
> > +       { CCI_REG8(0x0a67), 0x80 },
> > +       { CCI_REG8(0x0a4d), 0x4e },
> > +       { CCI_REG8(0x0a50), 0x00 },
> > +       { CCI_REG8(0x0a4f), 0x0c },
> > +       { CCI_REG8(0x0a66), 0x00 },
> > +       { CCI_REG8(0x00ca), 0x00 },
> > +       { CCI_REG8(0x00cc), 0x00 },
> > +       { CCI_REG8(0x00cd), 0x00 },
> > +       { CCI_REG8(0x0aa1), 0x00 },
> > +       { CCI_REG8(0x0aa2), 0xe0 },
> > +       { CCI_REG8(0x0aa3), 0x00 },
> > +       { CCI_REG8(0x0aa4), 0x40 },
> > +       { CCI_REG8(0x0a90), 0x03 },
> > +       { CCI_REG8(0x0a91), 0x0e },
> > +       { CCI_REG8(0x0a94), 0x80 },
> > +
> > +       /* standby */
> > +       { CCI_REG8(0x0af6), 0x20 },
> > +       { CCI_REG8(0x0b00), 0x91 },
> > +       { CCI_REG8(0x0b01), 0x17 },
> > +       { CCI_REG8(0x0b02), 0x01 },
> > +       { CCI_REG8(0x0b03), 0x00 },
> > +       { CCI_REG8(0x0b04), 0x01 },
> > +       { CCI_REG8(0x0b05), 0x17 },
> > +       { CCI_REG8(0x0b06), 0x01 },
> > +       { CCI_REG8(0x0b07), 0x00 },
> > +       { CCI_REG8(0x0ae9), 0x01 },
> > +       { CCI_REG8(0x0aea), 0x02 },
> > +       { CCI_REG8(0x0ae8), 0x53 },
> > +       { CCI_REG8(0x0ae8), 0x43 },
> > +
> > +       /* gain_partition */
> > +       { CCI_REG8(0x0af6), 0x30 },
> > +       { CCI_REG8(0x0b00), 0x08 },
> > +       { CCI_REG8(0x0b01), 0x0f },
> > +       { CCI_REG8(0x0b02), 0x00 },
> > +       { CCI_REG8(0x0b04), 0x1c },
> > +       { CCI_REG8(0x0b05), 0x24 },
> > +       { CCI_REG8(0x0b06), 0x00 },
> > +       { CCI_REG8(0x0b08), 0x30 },
> > +       { CCI_REG8(0x0b09), 0x40 },
> > +       { CCI_REG8(0x0b0a), 0x00 },
> > +       { CCI_REG8(0x0b0c), 0x0e },
> > +       { CCI_REG8(0x0b0d), 0x2a },
> > +       { CCI_REG8(0x0b0e), 0x00 },
> > +       { CCI_REG8(0x0b10), 0x0e },
> > +       { CCI_REG8(0x0b11), 0x2b },
> > +       { CCI_REG8(0x0b12), 0x00 },
> > +       { CCI_REG8(0x0b14), 0x0e },
> > +       { CCI_REG8(0x0b15), 0x23 },
> > +       { CCI_REG8(0x0b16), 0x00 },
> > +       { CCI_REG8(0x0b18), 0x0e },
> > +       { CCI_REG8(0x0b19), 0x24 },
> > +       { CCI_REG8(0x0b1a), 0x00 },
> > +       { CCI_REG8(0x0b1c), 0x0c },
> > +       { CCI_REG8(0x0b1d), 0x0c },
> > +       { CCI_REG8(0x0b1e), 0x00 },
> > +       { CCI_REG8(0x0b20), 0x03 },
> > +       { CCI_REG8(0x0b21), 0x03 },
> > +       { CCI_REG8(0x0b22), 0x00 },
> > +       { CCI_REG8(0x0b24), 0x0e },
> > +       { CCI_REG8(0x0b25), 0x0e },
> > +       { CCI_REG8(0x0b26), 0x00 },
> > +       { CCI_REG8(0x0b28), 0x03 },
> > +       { CCI_REG8(0x0b29), 0x03 },
> > +       { CCI_REG8(0x0b2a), 0x00 },
> > +       { CCI_REG8(0x0b2c), 0x12 },
> > +       { CCI_REG8(0x0b2d), 0x12 },
> > +       { CCI_REG8(0x0b2e), 0x00 },
> > +       { CCI_REG8(0x0b30), 0x08 },
> > +       { CCI_REG8(0x0b31), 0x08 },
> > +       { CCI_REG8(0x0b32), 0x00 },
> > +       { CCI_REG8(0x0b34), 0x14 },
> > +       { CCI_REG8(0x0b35), 0x14 },
> > +       { CCI_REG8(0x0b36), 0x00 },
> > +       { CCI_REG8(0x0b38), 0x10 },
> > +       { CCI_REG8(0x0b39), 0x10 },
> > +       { CCI_REG8(0x0b3a), 0x00 },
> > +       { CCI_REG8(0x0b3c), 0x16 },
> > +       { CCI_REG8(0x0b3d), 0x16 },
> > +       { CCI_REG8(0x0b3e), 0x00 },
> > +       { CCI_REG8(0x0b40), 0x10 },
> > +       { CCI_REG8(0x0b41), 0x10 },
> > +       { CCI_REG8(0x0b42), 0x00 },
> > +       { CCI_REG8(0x0b44), 0x19 },
> > +       { CCI_REG8(0x0b45), 0x19 },
> > +       { CCI_REG8(0x0b46), 0x00 },
> > +       { CCI_REG8(0x0b48), 0x16 },
> > +       { CCI_REG8(0x0b49), 0x16 },
> > +       { CCI_REG8(0x0b4a), 0x00 },
> > +       { CCI_REG8(0x0b4c), 0x19 },
> > +       { CCI_REG8(0x0b4d), 0x19 },
> > +       { CCI_REG8(0x0b4e), 0x00 },
> > +       { CCI_REG8(0x0b50), 0x16 },
> > +       { CCI_REG8(0x0b51), 0x16 },
> > +       { CCI_REG8(0x0b52), 0x00 },
> > +       { CCI_REG8(0x0b80), 0x01 },
> > +       { CCI_REG8(0x0b81), 0x00 },
> > +       { CCI_REG8(0x0b82), 0x00 },
> > +       { CCI_REG8(0x0b84), 0x00 },
> > +       { CCI_REG8(0x0b85), 0x00 },
> > +       { CCI_REG8(0x0b86), 0x00 },
> > +       { CCI_REG8(0x0b88), 0x01 },
> > +       { CCI_REG8(0x0b89), 0x6a },
> > +       { CCI_REG8(0x0b8a), 0x00 },
> > +       { CCI_REG8(0x0b8c), 0x00 },
> > +       { CCI_REG8(0x0b8d), 0x01 },
> > +       { CCI_REG8(0x0b8e), 0x00 },
> > +       { CCI_REG8(0x0b90), 0x01 },
> > +       { CCI_REG8(0x0b91), 0xf6 },
> > +       { CCI_REG8(0x0b92), 0x00 },
> > +       { CCI_REG8(0x0b94), 0x00 },
> > +       { CCI_REG8(0x0b95), 0x02 },
> > +       { CCI_REG8(0x0b96), 0x00 },
> > +       { CCI_REG8(0x0b98), 0x02 },
> > +       { CCI_REG8(0x0b99), 0xc4 },
> > +       { CCI_REG8(0x0b9a), 0x00 },
> > +       { CCI_REG8(0x0b9c), 0x00 },
> > +       { CCI_REG8(0x0b9d), 0x03 },
> > +       { CCI_REG8(0x0b9e), 0x00 },
> > +       { CCI_REG8(0x0ba0), 0x03 },
> > +       { CCI_REG8(0x0ba1), 0xd8 },
> > +       { CCI_REG8(0x0ba2), 0x00 },
> > +       { CCI_REG8(0x0ba4), 0x00 },
> > +       { CCI_REG8(0x0ba5), 0x04 },
> > +       { CCI_REG8(0x0ba6), 0x00 },
> > +       { CCI_REG8(0x0ba8), 0x05 },
> > +       { CCI_REG8(0x0ba9), 0x4d },
> > +       { CCI_REG8(0x0baa), 0x00 },
> > +       { CCI_REG8(0x0bac), 0x00 },
> > +       { CCI_REG8(0x0bad), 0x05 },
> > +       { CCI_REG8(0x0bae), 0x00 },
> > +       { CCI_REG8(0x0bb0), 0x07 },
> > +       { CCI_REG8(0x0bb1), 0x3e },
> > +       { CCI_REG8(0x0bb2), 0x00 },
> > +       { CCI_REG8(0x0bb4), 0x00 },
> > +       { CCI_REG8(0x0bb5), 0x06 },
> > +       { CCI_REG8(0x0bb6), 0x00 },
> > +       { CCI_REG8(0x0bb8), 0x0a },
> > +       { CCI_REG8(0x0bb9), 0x1a },
> > +       { CCI_REG8(0x0bba), 0x00 },
> > +       { CCI_REG8(0x0bbc), 0x09 },
> > +       { CCI_REG8(0x0bbd), 0x36 },
> > +       { CCI_REG8(0x0bbe), 0x00 },
> > +       { CCI_REG8(0x0bc0), 0x0e },
> > +       { CCI_REG8(0x0bc1), 0x66 },
> > +       { CCI_REG8(0x0bc2), 0x00 },
> > +       { CCI_REG8(0x0bc4), 0x10 },
> > +       { CCI_REG8(0x0bc5), 0x06 },
> > +       { CCI_REG8(0x0bc6), 0x00 },
> > +       { CCI_REG8(0x02c1), 0xe0 },
> > +       { CCI_REG8(0x0207), 0x04 },
> > +       { CCI_REG8(0x02c2), 0x10 },
> > +       { CCI_REG8(0x02c3), 0x74 },
> > +       { CCI_REG8(0x02c5), 0x09 },
> > +       { CCI_REG8(0x02c1), 0xe0 },
> > +       { CCI_REG8(0x0207), 0x04 },
> > +       { CCI_REG8(0x02c2), 0x10 },
> > +       { CCI_REG8(0x02c5), 0x09 },
> > +       { CCI_REG8(0x02c1), 0xe0 },
> > +       { CCI_REG8(0x0207), 0x04 },
> > +       { CCI_REG8(0x02c2), 0x10 },
> > +       { CCI_REG8(0x02c5), 0x09 },
> > +
> > +       /* auto load CH_GAIN */
> > +       { CCI_REG8(0x0aa1), 0x15 },
> > +       { CCI_REG8(0x0aa2), 0x50 },
> > +       { CCI_REG8(0x0aa3), 0x00 },
> > +       { CCI_REG8(0x0aa4), 0x09 },
> > +       { CCI_REG8(0x0a90), 0x25 },
> > +       { CCI_REG8(0x0a91), 0x0e },
> > +       { CCI_REG8(0x0a94), 0x80 },
> > +
> > +       /* ISP */
> > +       { CCI_REG8(0x0050), 0x00 },
> > +       { CCI_REG8(0x0089), 0x83 },
> > +       { CCI_REG8(0x005a), 0x40 },
> > +       { CCI_REG8(0x00c3), 0x35 },
> > +       { CCI_REG8(0x00c4), 0x80 },
> > +       { CCI_REG8(0x0080), 0x10 },
> > +       { CCI_REG8(0x0040), 0x12 },
> > +       { CCI_REG8(0x0053), 0x0a },
> > +       { CCI_REG8(0x0054), 0x44 },
> > +       { CCI_REG8(0x0055), 0x32 },
> > +       { CCI_REG8(0x0058), 0x89 },
> > +       { CCI_REG8(0x004a), 0x03 },
> > +       { CCI_REG8(0x0048), 0xf0 },
> > +       { CCI_REG8(0x0049), 0x0f },
> > +       { CCI_REG8(0x0041), 0x20 },
> > +       { CCI_REG8(0x0043), 0x0a },
> > +       { CCI_REG8(0x009d), 0x08 },
> > +       { CCI_REG8(0x0236), 0x40 },
> > +
> > +       /* gain */
> 
> Is the gain configurable? Is this analogue gain? digital gain? or
> colour
> balanace gains ?
> 
> 
> > +       { CCI_REG8(0x0204), 0x04 },
> > +       { CCI_REG8(0x0205), 0x00 },
> > +       { CCI_REG8(0x02b3), 0x00 },
> > +       { CCI_REG8(0x02b4), 0x00 },
> > +       { CCI_REG8(0x009e), 0x01 },
> > +       { CCI_REG8(0x009f), 0x94 },
> > +
> > +       /* auto load REG */
> > +       { CCI_REG8(0x0aa1), 0x10 },
> > +       { CCI_REG8(0x0aa2), 0xf8 },
> > +       { CCI_REG8(0x0aa3), 0x00 },
> > +       { CCI_REG8(0x0aa4), 0x1f },
> > +       { CCI_REG8(0x0a90), 0x11 },
> > +       { CCI_REG8(0x0a91), 0x0e },
> > +       { CCI_REG8(0x0a94), 0x80 },
> > +       { CCI_REG8(0x03fe), 0x00 },
> > +       { CCI_REG8(0x0a90), 0x00 },
> > +       { CCI_REG8(0x0a70), 0x00 },
> > +       { CCI_REG8(0x0a67), 0x00 },
> > +       { CCI_REG8(0x0af4), 0x29 },
> > +
> > +       /* DPHY */
> > +       { CCI_REG8(0x0d80), 0x07 },
> > +       { CCI_REG8(0x0dd3), 0x18 },
> > +
> > +       /* CISCTL_Reset */
> > +       { CCI_REG8(0x031c), 0x80 },
> > +       { CCI_REG8(0x03fe), 0x30 },
> > +       { CCI_REG8(0x0d17), 0x06 },
> > +       { CCI_REG8(0x03fe), 0x00 },
> > +       { CCI_REG8(0x0d17), 0x00 },
> > +       { CCI_REG8(0x031c), 0x93 },
> > +       { CCI_REG8(0x03fe), 0x00 },
> > +       { CCI_REG8(0x031c), 0x80 },
> > +       { CCI_REG8(0x03fe), 0x30 },
> > +       { CCI_REG8(0x0d17), 0x06 },
> > +       { CCI_REG8(0x03fe), 0x00 },
> > +       { CCI_REG8(0x0d17), 0x00 },
> > +       { CCI_REG8(0x031c), 0x93 },
> > +};
> > +
> > +struct gc05a2_mode {
> > +       u32 width;
> > +       u32 height;
> > +       const struct gc05a2_reg_list reg_list;
> > +
> > +       u32 hts; /* Horizontal timining size */
> > +       u32 vts_def; /* Default vertical timining size */
> > +       u32 vts_min; /* Min vertical timining size */
> > +};
> > +
> > +/* Declare modes in order, from biggest to smallest height. */
> > +static const struct gc05a2_mode gc05a2_modes[] = {
> > +       {
> > +               /* 2592*1944@30fps */
> > +               .width = GC05A2_NATIVE_WIDTH,
> > +               .height = GC05A2_NATIVE_HEIGHT,
> > +               .reg_list = {
> > +                       .num_of_regs = ARRAY_SIZE(mode_2592x1944),
> > +                       .regs = mode_2592x1944,
> > +               },
> > +               .hts = 3664,
> > +               .vts_def = 2032,
> > +               .vts_min = 2032,
> > +       },
> > +       {
> > +               /* 1280*720@60fps */
> > +               .width = 1280,
> > +               .height = 720,
> > +               .reg_list = {
> > +                       .num_of_regs = ARRAY_SIZE(mode_1280x720),
> > +                       .regs = mode_1280x720,
> > +               },
> > +               .hts = 3616,
> > +               .vts_def = 1032,
> > +               .vts_min = 1032,
> > +       },
> > +};
> > +
> > +static inline struct gc05a2 *to_gc05a2(struct v4l2_subdev *sd)
> > +{
> > +       return container_of(sd, struct gc05a2, sd);
> > +}
> > +
> > +static int gc05a2_power_on(struct device *dev)
> > +{
> > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +       int ret;
> > +
> > +       ret = regulator_bulk_enable(ARRAY_SIZE(gc05a2_supply_name),
> > +                                   gc05a2->supplies);
> > +       if (ret < 0) {
> > +               dev_err(gc05a2->dev, "failed to enable regulators:
> %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       ret = clk_prepare_enable(gc05a2->xclk);
> > +       if (ret < 0) {
> >
> +               regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name)
> ,
> > +                                      gc05a2->supplies);
> > +               dev_err(gc05a2->dev, "clk prepare enable
> failed\n");
> > +               return ret;
> > +       }
> > +
> > +       fsleep(GC05A2_SLEEP_US);
> > +
> > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 0);
> > +       fsleep(GC05A2_SLEEP_US);
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_power_off(struct device *dev)
> > +{
> > +       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +
> > +       clk_disable_unprepare(gc05a2->xclk);
> > +       gpiod_set_value_cansleep(gc05a2->reset_gpio, 1);
> > +       regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name),
> > +                              gc05a2->supplies);
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_enum_mbus_code(struct v4l2_subdev *sd,
> > +                                struct v4l2_subdev_state
> *sd_state,
> > +                                struct v4l2_subdev_mbus_code_enum
> *code)
> > +{
> > +       if (code->index > 0)
> > +               return -EINVAL;
> > +
> > +       code->code = GC05A2_MBUS_CODE;
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_enum_frame_size(struct v4l2_subdev *subdev,
> > +                                 struct v4l2_subdev_state
> *sd_state,
> > +                                 struct
> v4l2_subdev_frame_size_enum *fse)
> > +{
> > +       if (fse->code != GC05A2_MBUS_CODE)
> > +               return -EINVAL;
> > +
> > +       if (fse->index >= ARRAY_SIZE(gc05a2_modes))
> > +               return -EINVAL;
> > +
> > +       fse->min_width = gc05a2_modes[fse->index].width;
> > +       fse->max_width = gc05a2_modes[fse->index].width;
> > +       fse->min_height = gc05a2_modes[fse->index].height;
> > +       fse->max_height = gc05a2_modes[fse->index].height;
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_update_cur_mode_controls(struct gc05a2 *gc05a2,
> > +                                          const struct gc05a2_mode
> *mode)
> > +{
> > +       s64 exposure_max, h_blank;
> > +       int ret;
> > +
> > +       ret = __v4l2_ctrl_modify_range(gc05a2->vblank,
> > +                                      mode->vts_min - mode-
> >height,
> > +                                      GC05A2_VTS_MAX - mode-
> >height, 1,
> > +                                      mode->vts_def - mode-
> >height);
> > +       if (ret) {
> > +               dev_err(gc05a2->dev, "VB ctrl range update
> failed\n");
> > +               return ret;
> > +       }
> > +
> > +       h_blank = mode->hts - mode->width;
> > +       ret = __v4l2_ctrl_modify_range(gc05a2->hblank, h_blank,
> h_blank, 1,
> > +                                      h_blank);
> > +       if (ret) {
> > +               dev_err(gc05a2->dev, "HB ctrl range update
> failed\n");
> > +               return ret;
> > +       }
> > +
> > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > +       ret = __v4l2_ctrl_modify_range(gc05a2->exposure,
> GC05A2_EXP_MIN,
> > +                                      exposure_max,
> GC05A2_EXP_STEP,
> > +                                      exposure_max);
> > +       if (ret) {
> > +               dev_err(gc05a2->dev, "exposure ctrl range update
> failed\n");
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void gc05a2_update_pad_format(struct gc05a2 *gc08a3,
> > +                                    const struct gc05a2_mode
> *mode,
> > +                                    struct v4l2_mbus_framefmt
> *fmt)
> > +{
> > +       fmt->width = mode->width;
> > +       fmt->height = mode->height;
> > +       fmt->code = GC05A2_MBUS_CODE;
> > +       fmt->field = V4L2_FIELD_NONE;
> > +       fmt->colorspace = V4L2_COLORSPACE_RAW;
> > +       fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt-
> >colorspace);
> > +       fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > +       fmt->xfer_func = V4L2_XFER_FUNC_NONE;
> > +}
> > +
> > +static int gc05a2_set_format(struct v4l2_subdev *sd,
> > +                            struct v4l2_subdev_state *state,
> > +                            struct v4l2_subdev_format *fmt)
> > +{
> > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +       struct v4l2_mbus_framefmt *mbus_fmt;
> > +       struct v4l2_rect *crop;
> > +       const struct gc05a2_mode *mode;
> > +
> > +       mode = v4l2_find_nearest_size(gc05a2_modes,
> ARRAY_SIZE(gc05a2_modes),
> > +                                     width, height, fmt-
> >format.width,
> > +                                     fmt->format.height);
> > +
> > +       /* update crop info to subdev state */
> > +       crop = v4l2_subdev_state_get_crop(state, 0);
> > +       crop->width = mode->width;
> > +       crop->height = mode->height;
> > +
> > +       /* update fmt info to subdev state */
> > +       gc05a2_update_pad_format(gc05a2, mode, &fmt->format);
> > +       mbus_fmt = v4l2_subdev_state_get_format(state, 0);
> > +       *mbus_fmt = fmt->format;
> > +
> > +       if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > +               return 0;
> > +       gc05a2->cur_mode = mode;
> > +       gc05a2_update_cur_mode_controls(gc05a2, mode);
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_get_selection(struct v4l2_subdev *sd,
> > +                               struct v4l2_subdev_state *state,
> > +                               struct v4l2_subdev_selection *sel)
> > +{
> > +       switch (sel->target) {
> > +       case V4L2_SEL_TGT_CROP_DEFAULT:
> > +       case V4L2_SEL_TGT_CROP:
> > +               sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > +               break;
> > +       case V4L2_SEL_TGT_CROP_BOUNDS:
> > +               sel->r.top = 0;
> > +               sel->r.left = 0;
> > +               sel->r.width = GC05A2_NATIVE_WIDTH;
> > +               sel->r.height = GC05A2_NATIVE_HEIGHT;
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_init_state(struct v4l2_subdev *sd,
> > +                            struct v4l2_subdev_state *state)
> > +{
> > +       struct v4l2_subdev_format fmt = {
> > +               .which = V4L2_SUBDEV_FORMAT_TRY,
> > +               .pad = 0,
> > +               .format = {
> > +                       .code = GC05A2_MBUS_CODE,
> > +                       .width = gc05a2_modes[0].width,
> > +                       .height = gc05a2_modes[0].height,
> > +               },
> > +       };
> > +
> > +       gc05a2_set_format(sd, state, &fmt);
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_set_ctrl_hflip(struct gc05a2 *gc05a2, u32
> ctrl_val)
> > +{
> > +       int ret;
> > +       u64 val;
> > +
> > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> NULL);
> > +       if (ret) {
> > +               dev_err(gc05a2->dev, "read hflip register failed:
> %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > +                              GC05A2_FLIP_H_MASK,
> > +                              ctrl_val ? GC05A2_FLIP_H_MASK : 0,
> NULL);
> > +}
> > +
> > +static int gc05a2_set_ctrl_vflip(struct gc05a2 *gc05a2, u32
> ctrl_val)
> > +{
> > +       int ret;
> > +       u64 val;
> > +
> > +       ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> NULL);
> > +       if (ret) {
> > +               dev_err(gc05a2->dev, "read vflip register failed:
> %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > +                              GC05A2_FLIP_V_MASK,
> > +                              ctrl_val ? GC05A2_FLIP_V_MASK : 0,
> NULL);
> > +}
> > +
> > +static int gc05a2_test_pattern(struct gc05a2 *gc05a2, u32
> pattern_menu)
> > +{
> > +       u32 pattern;
> > +       int ret;
> > +
> > +       if (pattern_menu) {
> > +               switch (pattern_menu) {
> > +               case 1:
> > +               case 2:
> > +               case 3:
> > +               case 4:
> > +               case 5:
> > +               case 6:
> > +               case 7:
> > +                       pattern = pattern_menu << 4;
> > +                       break;
> > +
> > +               case 8:
> > +                       pattern = 0;
> > +                       break;
> > +
> > +               case 9:
> > +                       pattern = 4;
> > +                       break;
> > +
> > +               default:
> > +                       pattern = 0x00;
> > +                       break;
> > +               }
> 
> This is fairly terse. Can we add comments, or definitions for the
> types
> or such so that the above is easier to interpret?
> 
> > +
> > +               ret = cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_IDX,
> > +                               pattern, NULL);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               return cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_EN,
> > +                                GC05A2_TEST_PATTERN_EN, NULL);
> > +       } else {
> > +               return cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_EN,
> > +                                0x00, NULL);
> > +       }
> > +}
> > +
> > +static int gc05a2_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +       struct gc05a2 *gc05a2 =
> > +               container_of(ctrl->handler, struct gc05a2, ctrls);
> > +       int ret = 0;
> > +       s64 exposure_max;
> > +       struct v4l2_subdev_state *state;
> > +       const struct v4l2_mbus_framefmt *format;
> > +
> > +       state = v4l2_subdev_get_locked_active_state(&gc05a2->sd);
> > +       format = v4l2_subdev_state_get_format(state, 0);
> > +
> > +       if (ctrl->id == V4L2_CID_VBLANK) {
> > +               /* Update max exposure while meeting expected
> vblanking */
> > +               exposure_max = format->height + ctrl->val -
> GC05A2_EXP_MARGIN;
> > +               __v4l2_ctrl_modify_range(gc05a2->exposure,
> > +                                        gc05a2->exposure->minimum,
> > +                                        exposure_max, gc05a2-
> >exposure->step,
> > +                                        exposure_max);
> > +       }
> > +
> > +       /*
> > +        * Applying V4L2 control value only happens
> > +        * when power is on for streaming.
> > +        */
> > +       if (!pm_runtime_get_if_active(gc05a2->dev))
> > +               return 0;
> > +
> > +       switch (ctrl->id) {
> > +       case V4L2_CID_EXPOSURE:
> > +               ret = cci_write(gc05a2->regmap, GC05A2_EXP_REG,
> > +                               ctrl->val, NULL);
> > +               break;
> > +
> > +       case V4L2_CID_ANALOGUE_GAIN:
> > +               ret = cci_write(gc05a2->regmap, GC05A2_AGAIN_REG,
> > +                               ctrl->val, NULL);
> > +               break;
> > +
> > +       case V4L2_CID_VBLANK:
> > +               ret = cci_write(gc05a2->regmap,
> GC05A2_FRAME_LENGTH_REG,
> > +                               gc05a2->cur_mode->height + ctrl-
> >val, NULL);
> > +               break;
> > +
> > +       case V4L2_CID_HFLIP:
> > +               ret = gc05a2_set_ctrl_hflip(gc05a2, ctrl->val);
> > +               break;
> > +
> > +       case V4L2_CID_VFLIP:
> > +               ret = gc05a2_set_ctrl_vflip(gc05a2, ctrl->val);
> > +               break;
> > +
> > +       case V4L2_CID_TEST_PATTERN:
> > +               ret = gc05a2_test_pattern(gc05a2, ctrl->val);
> > +               break;
> > +
> > +       default:
> > +               break;
> > +       }
> > +
> > +       pm_runtime_put(gc05a2->dev);
> > +
> > +       return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops gc05a2_ctrl_ops = {
> > +       .s_ctrl = gc05a2_set_ctrl,
> > +};
> > +
> > +static int gc05a2_identify_module(struct gc05a2 *gc05a2)
> > +{
> > +       u64 val;
> > +       int ret;
> > +
> > +       if (gc05a2->identified)
> > +               return 0;
> > +
> > +       ret = cci_read(gc05a2->regmap, GC05A2_REG_CHIP_ID, &val,
> NULL);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (val != GC05A2_CHIP_ID) {
> > +               dev_err(gc05a2->dev, "chip id mismatch:
> 0x%x!=0x%llx",
> > +                       GC05A2_CHIP_ID, val);
> > +               return -ENXIO;
> > +       }
> > +
> > +       gc05a2->identified = true;
> > +
> > +       return 0;
> > +}
> > +
> > +static int gc05a2_start_streaming(struct gc05a2 *gc05a2)
> > +{
> > +       const struct gc05a2_mode *mode;
> > +       const struct gc05a2_reg_list *reg_list;
> > +       int ret;
> > +
> > +       ret = pm_runtime_resume_and_get(gc05a2->dev);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = gc05a2_identify_module(gc05a2);
> > +       if (ret)
> > +               goto err_rpm_put;
> > +
> > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > +                                 mode_table_common,
> > +                                 ARRAY_SIZE(mode_table_common),
> NULL);
> > +       if (ret)
> > +               goto err_rpm_put;
> > +
> > +       mode = gc05a2->cur_mode;
> > +       reg_list = &mode->reg_list;
> > +
> > +       ret = cci_multi_reg_write(gc05a2->regmap,
> > +                                 reg_list->regs, reg_list-
> >num_of_regs, NULL);
> > +       if (ret < 0)
> > +               goto err_rpm_put;
> > +
> > +       ret = __v4l2_ctrl_handler_setup(&gc05a2->ctrls);
> > +       if (ret < 0) {
> > +               dev_err(gc05a2->dev, "could not sync v4l2
> controls\n");
> > +               goto err_rpm_put;
> > +       }
> > +
> > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 1,
> NULL);
> > +       if (ret < 0) {
> > +               dev_err(gc05a2->dev, "write STREAMING_REG failed:
> %d\n", ret);
> > +               goto err_rpm_put;
> > +       }
> > +
> > +       return 0;
> > +
> > +err_rpm_put:
> > +       pm_runtime_put(gc05a2->dev);
> > +       return ret;
> > +}
> > +
> > +static int gc05a2_stop_streaming(struct gc05a2 *gc05a2)
> > +{
> > +       int ret;
> > +
> > +       ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 0,
> NULL);
> > +       if (ret < 0)
> > +               dev_err(gc05a2->dev, "could not sent stop streaming
> %d\n", ret);
> > +
> > +       pm_runtime_put(gc05a2->dev);
> > +       return ret;
> > +}
> > +
> > +static int gc05a2_s_stream(struct v4l2_subdev *subdev, int enable)
> > +{
> > +       struct gc05a2 *gc05a2 = to_gc05a2(subdev);
> > +       struct v4l2_subdev_state *state;
> > +       int ret;
> > +
> > +       state = v4l2_subdev_lock_and_get_active_state(subdev);
> > +
> > +       if (enable)
> > +               ret = gc05a2_start_streaming(gc05a2);
> > +       else
> > +               ret = gc05a2_stop_streaming(gc05a2);
> > +
> > +       v4l2_subdev_unlock_state(state);
> > +
> > +       return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops gc05a2_video_ops = {
> > +       .s_stream = gc05a2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops gc05a2_subdev_pad_ops = {
> > +       .enum_mbus_code = gc05a2_enum_mbus_code,
> > +       .enum_frame_size = gc05a2_enum_frame_size,
> > +       .get_fmt = v4l2_subdev_get_fmt,
> > +       .set_fmt = gc05a2_set_format,
> > +       .get_selection = gc05a2_get_selection,
> > +};
> > +
> > +static const struct v4l2_subdev_core_ops gc05a2_core_ops = {
> > +       .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > +       .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > +};
> > +
> > +static const struct v4l2_subdev_ops gc05a2_subdev_ops = {
> > +       .core = &gc05a2_core_ops,
> > +       .video = &gc05a2_video_ops,
> > +       .pad = &gc05a2_subdev_pad_ops,
> > +};
> > +
> > +static const struct v4l2_subdev_internal_ops gc05a2_internal_ops =
> {
> > +       .init_state = gc05a2_init_state,
> > +};
> > +
> > +static int gc05a2_get_regulators(struct device *dev, struct gc05a2
> *gc05a2)
> > +{
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(gc05a2_supply_name); i++)
> > +               gc05a2->supplies[i].supply = gc05a2_supply_name[i];
> > +
> > +       return devm_regulator_bulk_get(dev,
> ARRAY_SIZE(gc05a2_supply_name),
> > +                                      gc05a2->supplies);
> > +}
> > +
> > +static int gc05a2_parse_fwnode(struct gc05a2 *gc05a2)
> > +{
> > +       struct fwnode_handle *endpoint;
> > +       struct v4l2_fwnode_endpoint bus_cfg = {
> > +               .bus_type = V4L2_MBUS_CSI2_DPHY,
> > +       };
> > +       int ret;
> > +       struct device *dev = gc05a2->dev;
> > +
> > +       endpoint =
> > +               fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> 0,
> >
> +                                               FWNODE_GRAPH_ENDPOINT
> _NEXT);
> > +       if (!endpoint) {
> > +               dev_err(dev, "endpoint node not found\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
> > +       if (ret) {
> > +               dev_err(dev, "parsing endpoint node failed\n");
> > +               goto done;
> > +       }
> > +
> > +       ret = v4l2_link_freq_to_bitmap(dev,
> bus_cfg.link_frequencies,
> >
> +                                      bus_cfg.nr_of_link_frequencies
> ,
> > +                                      gc05a2_link_freq_menu_items,
> >
> +                                      ARRAY_SIZE(gc05a2_link_freq_me
> nu_items),
> > +                                      &gc05a2->link_freq_bitmap);
> > +       if (ret)
> > +               goto done;
> > +
> > +done:
> > +       v4l2_fwnode_endpoint_free(&bus_cfg);
> > +       fwnode_handle_put(endpoint);
> > +       return ret;
> > +}
> > +
> > +static u64 gc05a2_to_pixel_rate(u32 f_index)
> > +{
> > +       u64 pixel_rate =
> > +               gc05a2_link_freq_menu_items[f_index] * 2 *
> GC05A2_DATA_LANES;
> > +
> > +       return div_u64(pixel_rate, GC05A2_RGB_DEPTH);
> > +}
> > +
> > +static int gc05a2_init_controls(struct gc05a2 *gc05a2)
> > +{
> > +       struct i2c_client *client = v4l2_get_subdevdata(&gc05a2-
> >sd);
> > +       const struct gc05a2_mode *mode = &gc05a2_modes[0];
> > +       const struct v4l2_ctrl_ops *ops = &gc05a2_ctrl_ops;
> > +       struct v4l2_fwnode_device_properties props;
> > +       struct v4l2_ctrl_handler *ctrl_hdlr;
> > +       s64 exposure_max, h_blank;
> > +       int ret;
> > +
> > +       ctrl_hdlr = &gc05a2->ctrls;
> > +       ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> > +       if (ret)
> > +               return ret;
> > +
> > +       gc05a2->hflip = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > +                                         V4L2_CID_HFLIP, 0, 1, 1,
> 0);
> > +       gc05a2->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > +                                         V4L2_CID_VFLIP, 0, 1, 1,
> 0);
> > +       v4l2_ctrl_cluster(2, &gc05a2->hflip);
> > +
> > +       gc05a2->link_freq =
> > +       v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > +                              &gc05a2_ctrl_ops,
> > +                              V4L2_CID_LINK_FREQ,
> >
> +                              ARRAY_SIZE(gc05a2_link_freq_menu_items
> ) - 1,
> > +                              0,
> > +                              gc05a2_link_freq_menu_items);
> > +       if (gc05a2->link_freq)
> > +               gc05a2->link_freq->flags |=
> V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +       gc05a2->pixel_rate =
> > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > +                                 &gc05a2_ctrl_ops,
> > +                                 V4L2_CID_PIXEL_RATE, 0,
> > +                                 gc05a2_to_pixel_rate(0),
> > +                                 1,
> > +                                 gc05a2_to_pixel_rate(0));
> > +
> > +       gc05a2->vblank =
> > +               v4l2_ctrl_new_std(ctrl_hdlr,
> > +                                 &gc05a2_ctrl_ops,
> V4L2_CID_VBLANK,
> > +                                 mode->vts_min - mode->height,
> > +                                 GC05A2_VTS_MAX - mode->height, 1,
> > +                                 mode->vts_def - mode->height);
> > +
> > +       h_blank = mode->hts - mode->width;
> > +       gc05a2->hblank = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > +                                          V4L2_CID_HBLANK,
> h_blank, h_blank, 1,
> > +                                          h_blank);
> > +       if (gc05a2->hblank)
> > +               gc05a2->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +       v4l2_ctrl_new_std(ctrl_hdlr, &gc05a2_ctrl_ops,
> > +                         V4L2_CID_ANALOGUE_GAIN, GC05A2_AGAIN_MIN,
> > +                         GC05A2_AGAIN_MAX, GC05A2_AGAIN_STEP,
> > +                         GC05A2_AGAIN_MIN);
> > +
> > +       exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > +       gc05a2->exposure = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > +                                            V4L2_CID_EXPOSURE,
> GC05A2_EXP_MIN,
> > +                                            exposure_max,
> GC05A2_EXP_STEP,
> > +                                            exposure_max);
> > +
> > +       v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &gc05a2_ctrl_ops,
> > +                                    V4L2_CID_TEST_PATTERN,
> >
> +                                    ARRAY_SIZE(gc05a2_test_pattern_m
> enu) - 1,
> > +                                    0, 0,
> gc05a2_test_pattern_menu);
> > +
> > +       /* register properties to fwnode (e.g. rotation,
> orientation) */
> > +       ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > +       if (ret)
> > +               goto error_ctrls;
> > +
> > +       ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, ops,
> &props);
> > +       if (ret)
> > +               goto error_ctrls;
> > +
> > +       if (ctrl_hdlr->error) {
> > +               ret = ctrl_hdlr->error;
> > +               goto error_ctrls;
> > +       }
> > +
> > +       gc05a2->sd.ctrl_handler = ctrl_hdlr;
> > +
> > +       return 0;
> > +
> > +error_ctrls:
> > +       v4l2_ctrl_handler_free(ctrl_hdlr);
> > +
> > +       return ret;
> > +}
> > +
> > +static int gc05a2_probe(struct i2c_client *client)
> > +{
> > +       struct device *dev = &client->dev;
> > +       struct gc05a2 *gc05a2;
> > +       int ret;
> > +
> > +       gc05a2 = devm_kzalloc(dev, sizeof(*gc05a2), GFP_KERNEL);
> > +       if (!gc05a2)
> > +               return -ENOMEM;
> > +
> > +       gc05a2->dev = dev;
> > +
> > +       ret = gc05a2_parse_fwnode(gc05a2);
> > +       if (ret)
> > +               return ret;
> > +
> > +       gc05a2->regmap = devm_cci_regmap_init_i2c(client, 16);
> > +       if (IS_ERR(gc05a2->regmap))
> > +               return dev_err_probe(dev, PTR_ERR(gc05a2->regmap),
> > +                                    "failed to init CCI\n");
> > +
> > +       gc05a2->xclk = devm_clk_get(dev, NULL);
> > +       if (IS_ERR(gc05a2->xclk))
> > +               return dev_err_probe(dev, PTR_ERR(gc05a2->xclk),
> > +                                    "failed to get xclk\n");
> > +
> > +       ret = clk_set_rate(gc05a2->xclk, GC05A2_DEFAULT_CLK_FREQ);
> > +       if (ret)
> > +               return dev_err_probe(dev, ret,
> > +                                    "failed to set xclk
> frequency\n");
> > +
> > +       ret = gc05a2_get_regulators(dev, gc05a2);
> > +       if (ret < 0)
> > +               return dev_err_probe(dev, ret,
> > +                                    "failed to get regulators\n");
> > +
> > +       gc05a2->reset_gpio = devm_gpiod_get(dev, "reset",
> GPIOD_OUT_LOW);
> > +       if (IS_ERR(gc05a2->reset_gpio))
> > +               return dev_err_probe(dev, PTR_ERR(gc05a2-
> >reset_gpio),
> > +                                    "failed to get gpio\n");
> > +
> > +       v4l2_i2c_subdev_init(&gc05a2->sd, client,
> &gc05a2_subdev_ops);
> > +       gc05a2->sd.internal_ops = &gc05a2_internal_ops;
> > +       gc05a2->cur_mode = &gc05a2_modes[0];
> > +
> > +       ret = gc05a2_init_controls(gc05a2);
> > +       if (ret)
> > +               return dev_err_probe(dev, ret,
> > +                                    "failed to init controls\n");
> > +
> > +       gc05a2->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > +                           V4L2_SUBDEV_FL_HAS_EVENTS;
> > +       gc05a2->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +       gc05a2->sd.dev = &client->dev;
> > +       gc05a2->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > +       ret = media_entity_pads_init(&gc05a2->sd.entity, 1,
> &gc05a2->pad);
> > +       if (ret < 0) {
> > +               dev_err(dev, "could not register media entity\n");
> > +               goto err_v4l2_ctrl_handler_free;
> > +       }
> > +
> > +       gc05a2->sd.state_lock = gc05a2->ctrls.lock;
> > +       ret = v4l2_subdev_init_finalize(&gc05a2->sd);
> > +       if (ret < 0) {
> > +               dev_err(dev, "v4l2 subdev init error: %d\n", ret);
> > +               goto err_media_entity_cleanup;
> > +       }
> > +
> > +       pm_runtime_set_active(gc05a2->dev);
> > +       pm_runtime_enable(gc05a2->dev);
> > +       pm_runtime_set_autosuspend_delay(gc05a2->dev, 1000);
> > +       pm_runtime_use_autosuspend(gc05a2->dev);
> > +       pm_runtime_idle(gc05a2->dev);
> > +
> > +       ret = v4l2_async_register_subdev_sensor(&gc05a2->sd);
> > +       if (ret < 0) {
> > +               dev_err(dev, "could not register v4l2 device\n");
> > +               goto err_rpm;
> > +       }
> > +
> > +       return 0;
> > +
> > +err_rpm:
> > +       pm_runtime_disable(gc05a2->dev);
> > +       v4l2_subdev_cleanup(&gc05a2->sd);
> > +
> > +err_media_entity_cleanup:
> > +       media_entity_cleanup(&gc05a2->sd.entity);
> > +
> > +err_v4l2_ctrl_handler_free:
> > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > +
> > +       return ret;
> > +}
> > +
> > +static void gc05a2_remove(struct i2c_client *client)
> > +{
> > +       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +       struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +
> > +       v4l2_async_unregister_subdev(&gc05a2->sd);
> > +       v4l2_subdev_cleanup(sd);
> > +       media_entity_cleanup(&gc05a2->sd.entity);
> > +       v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > +
> > +       pm_runtime_disable(&client->dev);
> > +       if (!pm_runtime_status_suspended(&client->dev))
> > +               gc05a2_power_off(gc05a2->dev);
> > +       pm_runtime_set_suspended(&client->dev);
> > +}
> > +
> > +static const struct of_device_id gc05a2_of_match[] = {
> > +       { .compatible = "galaxycore,gc05a2" },
> > +       {}
> > +};
> > +MODULE_DEVICE_TABLE(of, gc05a2_of_match);
> > +
> > +static DEFINE_RUNTIME_DEV_PM_OPS(gc05a2_pm_ops,
> > +                                gc05a2_power_off,
> > +                                gc05a2_power_on,
> > +                                NULL);
> > +
> > +static struct i2c_driver gc05a2_i2c_driver = {
> > +       .driver = {
> > +               .of_match_table = gc05a2_of_match,
> > +               .pm = pm_ptr(&gc05a2_pm_ops),
> > +               .name  = "gc05a2",
> > +       },
> > +       .probe = gc05a2_probe,
> > +       .remove = gc05a2_remove,
> > +};
> > +module_i2c_driver(gc05a2_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("GalaxyCore gc05a2 Camera driver");
> > +MODULE_AUTHOR("Zhi Mao <zhi.mao@mediatek.com>");
> > +MODULE_LICENSE("GPL");
> > -- 
> > 2.25.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] perf: arm_cspmu: Don't touch interrupt registers if no interrupt was assigned
From: Robin Murphy @ 2024-04-08 12:05 UTC (permalink / raw)
  To: Ilkka Koskinen
  Cc: Besar Wicaksono, Suzuki K Poulose, Will Deacon, Mark Rutland,
	Raag Jadav, linux-arm-kernel, linux-kernel
In-Reply-To: <89d7ff41-ce2a-fd95-ebfc-4df914efd4c8@os.amperecomputing.com>

On 2024-04-05 11:33 pm, Ilkka Koskinen wrote:
> 
> On Fri, 5 Apr 2024, Robin Murphy wrote:
>> On 2024-03-07 7:31 pm, Ilkka Koskinen wrote:
>>> The driver enabled and disabled interrupts even if no interrupt was
>>> assigned to the device.
>>
>> Why's that a concern - if the interrupt isn't routed anywhere, surely 
>> it makes no difference what happens at the source end?
> 
> The issue is that we have two PMUs attached to the same interrupt line.
> Unfortunately, I just don't seem to find time to add support for shared 
> interrupts to the cspmu driver. Meanwhile, I assigned the interrupt to 
> one of the PMUs while the other one has zero in the APMT table.

I suspected something like that ;)

> Without 
> the patch, I can trigger "ghost interrupt" in the latter PMU.

An occasional spurious interrupt should be no big deal. If it ends up as 
a screaming spurious interrupt because we never handle the overflow 
condition on the "other" PMU, then what matters most is that we never 
handle the overflow, thus the "other" PMU is still useless since you 
can't assume the user is going to read it frequently enough to avoid 
losing information and getting nonsense counts back. So this hack really 
isn't a viable solution for anything.

Thanks,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 1/2] media: dt-bindings: nxp,imx8-jpeg: Add clocks entries
From: Mirela Rabulea @ 2024-04-08 12:06 UTC (permalink / raw)
  To: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam, festevam,
	alexander.stein, Frank.li, ming.qian
  Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
	hverkuil, linux-media, imx, linux-kernel

From: Fabio Estevam <festevam@denx.de>

The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
the PER and IPG clocks to be functional, so add the clock entries.

This also fixes the following schema warning:

imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
        from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#

Signed-off-by: Fabio Estevam <festevam@denx.de>
Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes since v5:
- Remove one extra ":" from subject (thanks Fabio!)
- Use <festevam@denx.de> address for both Author and Signed-of (Fabio's feedback)

Changes since v4:
- Remove redundant description and update subject prefix (per Krzysztof's feddback)

Changes since v3:
- Add items for clocks (per Krzysztof's feddback)
- Add description for clocks (per Conor's feddback to the other similar patch from Alexander)
- Add "media:" to the subject
- Add Mirela's signed-off
- For the similar patches that were sent for this issue, should Co-developed-by/Signed-off-by be added? Alexander Stein? Frank Li?

Changes since v2:
- Remove clock-names. (Mirela)

 .../devicetree/bindings/media/nxp,imx8-jpeg.yaml      | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 3d9d1db37040..2be30c5fdc83 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -31,6 +31,11 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description: AXI DMA engine clock for fetching JPEG bitstream from memory (per)
+      - description: IP bus clock for register access (ipg)
+
   interrupts:
     description: |
       There are 4 slots available in the IP, which the driver may use
@@ -49,6 +54,7 @@ properties:
 required:
   - compatible
   - reg
+  - clocks
   - interrupts
   - power-domains
 
@@ -56,12 +62,15 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/clock/imx8-lpcg.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/firmware/imx/rsrc.h>
 
     jpegdec: jpegdec@58400000 {
         compatible = "nxp,imx8qxp-jpgdec";
         reg = <0x58400000 0x00050000 >;
+        clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+                 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
         interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
@@ -76,6 +85,8 @@ examples:
     jpegenc: jpegenc@58450000 {
         compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
         reg = <0x58450000 0x00050000 >;
+        clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+                 <&img_jpeg__lpcg IMX_LPCG_CLK_4>;
         interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
                      <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 2/2] arm64: dts: imx8-ss-img: Remove JPEG clock-names
From: Mirela Rabulea @ 2024-04-08 12:06 UTC (permalink / raw)
  To: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam, festevam,
	alexander.stein, Frank.li, ming.qian
  Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
	hverkuil, linux-media, imx, linux-kernel
In-Reply-To: <20240408120654.1196880-1-mirela.rabulea@nxp.com>

From: Fabio Estevam <festevam@denx.de>

Per nxp,imx8-jpeg.yaml, the clock-names entry is not valid.

Remove them.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v5:
- Use <festevam@denx.de> address for both Author and Signed-of (Fabio's feedback)
Changes since v4:
- None
Changes since v3:
- Just added "imx8-ss-img:" in the subject

Changes since v2:
- Newly introduced.

 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index e7783cc2d830..77d2928997b4 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -21,7 +21,6 @@ jpegdec: jpegdec@58400000 {
 		interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
 			 <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
-		clock-names = "per", "ipg";
 		assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
 				  <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
 		assigned-clock-rates = <200000000>, <200000000>;
@@ -35,7 +34,6 @@ jpegenc: jpegenc@58450000 {
 		interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
 			 <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
-		clock-names = "per", "ipg";
 		assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
 				  <&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
 		assigned-clock-rates = <200000000>, <200000000>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 2/4] selftests/arm: Add signal tests
From: Mark Brown @ 2024-04-08 12:12 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Dev Jain, shuah, linux-arm-kernel, linux-kselftest, linux-kernel,
	Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
	Catalin.Marinas, will, mark.rutland
In-Reply-To: <1ce0e9c7-0bd3-47c1-893c-3ea5aa95fef5@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 1135 bytes --]

On Sun, Apr 07, 2024 at 02:28:06AM +0500, Muhammad Usama Anjum wrote:
> On 4/5/24 1:44 PM, Dev Jain wrote:

> > +	ksft_print_msg("%s :: %s\n", current->name, current->descr);
> > +	if (test_setup(current) && test_init(current)) {
> > +		test_run(current);
> > +		test_cleanup(current);
> > +	}
> > +	test_result(current);
> > +
> > +	return current->result;
> > +}

> This test isn't TAP compliant. Please make this and all tests TAP
> compilant. The 1/4 patch has example of TAP usage.

It's based on the 64 bit version of these tests which are also not TAP
compliant.  TBH I'm not sure how worthwile it is to fix at all given
that they're all single test executables anyway, if it does get fixed
it'd be good to do the arm64 ones as well.

> > +	} else {
> > +		fprintf(stdout, "==>> completed. FAIL(0)\n");
> > +		td->result = KSFT_FAIL;
> > +	}
> > +
> > +	if (force_exit)
> > +		exit(td->result);
> > +}
> > +

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH V2 1/2] arm64: dts: imx8mp-msc-sm2s: do not write i2c pinctrl reserved bit
From: Ian Ray @ 2024-04-08 12:23 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
  Cc: Ian Ray, devicetree, imx, linux-arm-kernel, linux-kernel

Better not to write to the reserved bit.

Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
 .../boot/dts/freescale/imx8mp-msc-sm2s.dtsi   | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
index 61c2a63efc6d..940bdbe115a3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
@@ -602,38 +602,38 @@ pinctrl_flexspi0: flexspi0grp {
 
 	pinctrl_i2c1: i2c1grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c2>;
 	};
 
 	pinctrl_i2c2: i2c2grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA		0x400001c2>;
 	};
 
 	pinctrl_i2c3: i2c3grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA		0x400001c2>;
 	};
 
 	pinctrl_i2c4: i2c4grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA		0x400001c2>;
 	};
 
 	pinctrl_i2c5: i2c5grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_SPDIF_TX__I2C5_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_SPDIF_RX__I2C5_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_SPDIF_TX__I2C5_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_SPDIF_RX__I2C5_SDA		0x400001c2>;
 	};
 
 	pinctrl_i2c6: i2c6grp {
 		fsl,pins =
-			<MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL		0x400001c3>,
-			<MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA		0x400001c3>;
+			<MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL		0x400001c2>,
+			<MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA		0x400001c2>;
 	};
 
 	pinctrl_lcd0_backlight: lcd0-backlightgrp {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 4/4] selftests: Add build infrastructure along with README
From: Mark Brown @ 2024-04-08 12:24 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Dev Jain, shuah, linux-arm-kernel, linux-kselftest, linux-kernel,
	Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
	Catalin.Marinas, will, mark.rutland
In-Reply-To: <ab271c32-dca8-413f-b97e-c8ceb6d7924c@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 904 bytes --]

On Sun, Apr 07, 2024 at 02:15:15AM +0500, Muhammad Usama Anjum wrote:
> On 4/5/24 1:44 PM, Dev Jain wrote:

> > @@ -0,0 +1,31 @@
> > +KSelfTest ARM
> > +===============
> > +
> > +- This is a series of compatibility tests, wherein the source files are
> > +  built statically into a 32 bit ELF; they should pass on both 32 and 64
> > +  bit kernels. They are not built or run but just skipped completely when
> > +  env-variable ARCH is found to be different than 'arm64' or 'arm' and
> > +  `uname -m` reports other than 'aarch64', 'armv7l' or 'armv8l'.
> > +
> > +- Please ensure that the test kernel is built with CONFIG_COMPAT enabled.

> Please create a config file and put all the per-requisite configurations in
> that. For example, look at tools/testing/selftests/mm/config

Note that arm and arm64 are different architectures, and the kernel
config stuff only applies when building for arm64...

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH V2 2/2] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Ian Ray @ 2024-04-08 12:23 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
  Cc: Ian Ray, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240408122321.464-1-ian.ray@gehealthcare.com>

Add i2c{1,6} sda-/scl-gpios with the corresponding pinmux entries.

Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
 .../boot/dts/freescale/imx8mp-msc-sm2s.dtsi   | 22 +++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
index 940bdbe115a3..aeb557fe9dd6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
@@ -200,8 +200,11 @@ ethphy1: ethernet-phy@1 {
 };
 
 &i2c1 {
-	pinctrl-names = "default";
+	pinctrl-names = "default", "gpio";
 	pinctrl-0 = <&pinctrl_i2c1>;
+	pinctrl-1 = <&pinctrl_i2c1_gpio>;
+	scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 	clock-frequency = <400000>;
 	status = "okay";
 
@@ -241,8 +244,11 @@ &i2c5 {
 };
 
 &i2c6 {
-	pinctrl-names = "default";
+	pinctrl-names = "default", "gpio";
 	pinctrl-0 = <&pinctrl_i2c6>;
+	pinctrl-1 = <&pinctrl_i2c6_gpio>;
+	scl-gpios = <&gpio3 19 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+	sda-gpios = <&gpio3 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
 	clock-frequency = <400000>;
 	status = "okay";
 
@@ -606,6 +612,12 @@ pinctrl_i2c1: i2c1grp {
 			<MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA		0x400001c2>;
 	};
 
+	pinctrl_i2c1_gpio: i2c1gpiogrp {
+		fsl,pins =
+			<MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14		0x1c2>,
+			<MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15		0x1c2>;
+	};
+
 	pinctrl_i2c2: i2c2grp {
 		fsl,pins =
 			<MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL		0x400001c2>,
@@ -636,6 +648,12 @@ pinctrl_i2c6: i2c6grp {
 			<MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA		0x400001c2>;
 	};
 
+	pinctrl_i2c6_gpio: i2c6gpiogrp {
+		fsl,pins =
+			<MX8MP_IOMUXC_SAI5_RXFS__GPIO3_IO19		0x1c2>,
+			<MX8MP_IOMUXC_SAI5_RXC__GPIO3_IO20		0x1c2>;
+	};
+
 	pinctrl_lcd0_backlight: lcd0-backlightgrp {
 		fsl,pins =
 			<MX8MP_IOMUXC_GPIO1_IO05__GPIO1_IO05		0x41>;
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v6 1/2] media: dt-bindings: nxp,imx8-jpeg: Add clocks entries
From: Krzysztof Kozlowski @ 2024-04-08 12:31 UTC (permalink / raw)
  To: Mirela Rabulea, shawnguo, robh+dt, krzysztof.kozlowski+dt,
	festevam, festevam, alexander.stein, Frank.li, ming.qian
  Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
	hverkuil, linux-media, imx, linux-kernel
In-Reply-To: <20240408120654.1196880-1-mirela.rabulea@nxp.com>

On 08/04/2024 14:06, Mirela Rabulea wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
> the PER and IPG clocks to be functional, so add the clock entries.
> 
> This also fixes the following schema warning:
> 
> imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---

This is like third version today? Give people chance to review your code
and wait 24h before postings.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 1/2] media: dt-bindings: nxp,imx8-jpeg: Add clocks entries
From: Krzysztof Kozlowski @ 2024-04-08 12:32 UTC (permalink / raw)
  To: Mirela Rabulea, shawnguo, robh+dt, krzysztof.kozlowski+dt,
	festevam, festevam, alexander.stein, Frank.li, ming.qian
  Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
	hverkuil, linux-media, imx, linux-kernel
In-Reply-To: <20240408120654.1196880-1-mirela.rabulea@nxp.com>

On 08/04/2024 14:06, Mirela Rabulea wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
> the PER and IPG clocks to be functional, so add the clock entries.
> 
> This also fixes the following schema warning:
> 
> imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/1] arm64: dts: rockchip: enable onboard spi flash for rock-3a
From: Chukun Pan @ 2024-04-08 12:40 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Conor Dooley, Krzysztof Kozlowski, Rob Herring, linux-arm-kernel,
	linux-rockchip, linux-kernel, devicetree, Chukun Pan

There is a mx25u12835f spi flash on this board, enable it.

[    2.525805] spi-nor spi4.0: mx25u12835f (16384 Kbytes)

Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
index a5e974ea659e..d8738cc47c73 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
@@ -757,6 +757,18 @@ &sdmmc2 {
 	status = "okay";
 };
 
+&sfc {
+	status = "okay";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0x0>;
+		spi-max-frequency = <50000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <1>;
+	};
+};
+
 &tsadc {
 	rockchip,hw-tshut-mode = <1>;
 	rockchip,hw-tshut-polarity = <0>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox