Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/2] mailbox: arm_mhuv3: Add driver
From: Jassi Brar @ 2024-04-08  1:14 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <20240404062347.3219795-3-cristian.marussi@arm.com>

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.
>
> 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.

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

> +
> +/* 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.

> +       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.

> +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.

> +       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];
> +       /* ------------------ */
> +       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 ?


> +       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.

> +       /* 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);
> +}
> +
> +static int mhuv3_doorbell_rx_startup(struct mhuv3 *mhu, struct mbox_chan *chan)
> +{
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       /* Unmask Channel Transfer events */
> +       writel_relaxed(BIT(priv->doorbell), &mhu->mbx->dbcw[priv->ch_idx].msk_clr);
> +
> +       return 0;
> +}
> +
> +static void mhuv3_doorbell_rx_shutdown(struct mhuv3 *mhu,
> +                                      struct mbox_chan *chan)
> +{
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       /* Mask Channel Transfer events */
> +       writel_relaxed(BIT(priv->doorbell), &mhu->mbx->dbcw[priv->ch_idx].msk_set);
> +}
> +
> +static void mhuv3_doorbell_rx_complete(struct mhuv3 *mhu, struct mbox_chan *chan)
> +{
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       /* Clearing the pending transfer generates the Channel Transfer Ack */
> +       writel_relaxed(BIT(priv->doorbell), &mhu->mbx->dbcw[priv->ch_idx].clr);
> +}
> +
> +static int mhuv3_doorbell_last_tx_done(struct mhuv3 *mhu,
> +                                      struct mbox_chan *chan)
> +{
> +       int done;
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       done = !(readl_relaxed(&mhu->pbx->dbcw[priv->ch_idx].st) &
> +                BIT(priv->doorbell));
> +       if (done) {
> +               unsigned long flags;
> +               struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> +
> +               /* Take care to clear the pending doorbell also when polling */
> +               spin_lock_irqsave(&e->pending_lock, flags);
> +               e->pending_db[priv->ch_idx] &= ~BIT(priv->doorbell);
> +               spin_unlock_irqrestore(&e->pending_lock, flags);
> +       }
> +
> +       return done;
> +}
> +
> +static int mhuv3_doorbell_send_data(struct mhuv3 *mhu, struct mbox_chan *chan,
> +                                   void *arg)
> +{
> +       int ret = 0;
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&e->pending_lock, flags);
> +       /* Only one in-flight Transfer is allowed per-doorbell */
> +       if (!(e->pending_db[priv->ch_idx] & BIT(priv->doorbell))) {
> +               e->pending_db[priv->ch_idx] |= BIT(priv->doorbell);
> +               writel_relaxed(BIT(priv->doorbell),
> +                              &mhu->pbx->dbcw[priv->ch_idx].set);
> +       } else {
> +               ret = -EBUSY;
> +       }
> +       spin_unlock_irqrestore(&e->pending_lock, flags);
> +
> +       return ret;
> +}
> +
> +static const struct mhuv3_protocol_ops mhuv3_doorbell_ops = {
> +       .tx_startup = mhuv3_doorbell_tx_startup,
> +       .tx_shutdown = mhuv3_doorbell_tx_shutdown,
> +       .rx_startup = mhuv3_doorbell_rx_startup,
> +       .rx_shutdown = mhuv3_doorbell_rx_shutdown,
> +       .rx_complete = mhuv3_doorbell_rx_complete,
> +       .last_tx_done = mhuv3_doorbell_last_tx_done,
> +       .send_data = mhuv3_doorbell_send_data,
> +};
> +
> +/* Sender and receiver mailbox ops */
> +static bool mhuv3_sender_last_tx_done(struct mbox_chan *chan)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       return priv->ops->last_tx_done(mhu, chan);
> +}
> +
> +static int mhuv3_sender_send_data(struct mbox_chan *chan, void *data)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       if (!priv->ops->last_tx_done(mhu, chan))
> +               return -EBUSY;
> +
> +       return priv->ops->send_data(mhu, chan, data);
> +}
> +
> +static int mhuv3_sender_startup(struct mbox_chan *chan)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       if (priv->ops->tx_startup)
> +               priv->ops->tx_startup(mhu, chan);
> +
> +       return 0;
> +}
> +
> +static void mhuv3_sender_shutdown(struct mbox_chan *chan)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       if (priv->ops->tx_shutdown)
> +               priv->ops->tx_shutdown(mhu, chan);
> +}
> +
> +static const struct mbox_chan_ops mhuv3_sender_ops = {
> +       .send_data = mhuv3_sender_send_data,
> +       .startup = mhuv3_sender_startup,
> +       .shutdown = mhuv3_sender_shutdown,
> +       .last_tx_done = mhuv3_sender_last_tx_done,
> +};
> +
> +static int mhuv3_receiver_startup(struct mbox_chan *chan)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       return priv->ops->rx_startup(mhu, chan);
> +}
> +
> +static void mhuv3_receiver_shutdown(struct mbox_chan *chan)
> +{
> +       struct mhuv3 *mhu = mhu_from_mbox(chan->mbox);
> +       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +       priv->ops->rx_shutdown(mhu, chan);
> +}
> +
> +static int mhuv3_receiver_send_data(struct mbox_chan *chan, void *data)
> +{
> +       dev_err(chan->mbox->dev,
> +               "Trying to transmit on a MBX MHUv3 frame\n");
> +       return -EIO;
> +}
> +
> +static bool mhuv3_receiver_last_tx_done(struct mbox_chan *chan)
> +{
> +       dev_err(chan->mbox->dev, "Trying to Tx poll on a MBX MHUv3 frame\n");
> +       return true;
> +}
> +
> +static const struct mbox_chan_ops mhuv3_receiver_ops = {
> +       .send_data = mhuv3_receiver_send_data,
> +       .startup = mhuv3_receiver_startup,
> +       .shutdown = mhuv3_receiver_shutdown,
> +       .last_tx_done = mhuv3_receiver_last_tx_done,
> +};
> +
> +static struct mbox_chan *mhuv3_dbe_mbox_of_xlate(struct mhuv3 *mhu,
> +                                                unsigned int channel,
> +                                                unsigned int doorbell)
> +{
> +       struct mbox_controller *mbox = &mhu->mbox;
> +       struct mbox_chan *chans = mbox->chans;
> +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> +
> +       if (channel >= e->max_chans || doorbell >= MHUV3_STAT_BITS) {
> +               dev_err(mbox->dev, "Couldn't xlate to a valid channel (%d: %d)\n",
> +                       channel, doorbell);
> +               return ERR_PTR(-ENODEV);
> +       }
> +
> +       return &chans[e->base_ch_idx + channel * MHUV3_STAT_BITS + doorbell];
> +}
> +
> +static void mhuv3_dbe_combined_irq_setup(struct mhuv3 *mhu)
> +{
> +       int i;
> +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> +
> +       if (mhu->frame == PBX_FRAME) {
> +               struct pdbcw_page __iomem *dbcw = mhu->pbx->dbcw;
> +
> +               for (i = 0; i < e->max_chans; i++) {
> +                       writel_relaxed_bitfield(0x1, &dbcw[i].int_clr, tfr_ack);
> +                       writel_relaxed_bitfield(0x0, &dbcw[i].int_en, tfr_ack);
> +                       writel_relaxed_bitfield(0x1, &dbcw[i].ctrl, comb_en);
> +               }
> +       } else {
> +               struct mdbcw_page __iomem *dbcw = mhu->mbx->dbcw;
> +
> +               for (i = 0; i < e->max_chans; i++) {
> +                       writel_relaxed(0xFFFFFFFF, &dbcw[i].clr);
> +                       writel_relaxed(0xFFFFFFFF, &dbcw[i].msk_set);
> +                       writel_relaxed_bitfield(0x1, &dbcw[i].ctrl, comb_en);
> +               }
> +       }
> +}
> +
> +static int mhuv3_dbe_channels_init(struct mhuv3 *mhu)
> +{
> +       int i;
> +       struct mhuv3_extension *e = mhu->ext[DBE_EXT];
> +       struct mbox_controller *mbox = &mhu->mbox;
> +       struct mbox_chan *chans;
> +
> +       chans = mbox->chans + mbox->num_chans;
> +       e->base_ch_idx = mbox->num_chans;
> +       for (i = 0; i < e->max_chans; i++) {
> +               int k;
> +               struct mhuv3_mbox_chan_priv *priv;
> +
> +               for (k = 0; k < MHUV3_STAT_BITS; k++) {
> +                       priv = devm_kmalloc(mbox->dev, sizeof(*priv), GFP_KERNEL);
> +                       if (!priv)
> +                               return -ENOMEM;
> +
> +                       priv->ch_idx = i;
> +                       priv->ops = &mhuv3_doorbell_ops;
> +                       priv->doorbell = k;
> +                       chans++->con_priv = priv;
> +                       mbox->num_chans++;
> +               }
> +       }
> +
> +       spin_lock_init(&e->pending_lock);
> +
> +       return 0;
> +}
> +
> +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.

> +               if (channel >= e->max_chans) {
> +                       dev_err(dev, "Invalid %s channel:%d\n",
> +                               mhuv3_str[mhu->frame], channel);
> +                       break;
> +               }
> +
> +               if (mhu->frame == PBX_FRAME) {
> +                       unsigned long flags;
> +                       u32 active_dbs, fired_dbs;
> +
> +                       st = readl_relaxed_bitfield(&mhu->pbx->dbcw[channel].int_st,
> +                                                   tfr_ack);
> +                       if (!st) {
> +                               dev_warn(dev, "Spurios IRQ on %s channel:%d\n",
> +                                        mhuv3_str[mhu->frame], channel);
> +                               continue;
> +                       }
> +
> +                       active_dbs = readl_relaxed(&mhu->pbx->dbcw[channel].st);
> +                       spin_lock_irqsave(&e->pending_lock, flags);
> +                       fired_dbs = e->pending_db[channel] & ~active_dbs;
> +                       if (fired_dbs) {
> +                               db = __builtin_ctz(fired_dbs);
> +                               e->pending_db[channel] &= ~BIT(db);
> +                               fired_dbs &= ~BIT(db);
> +                       }
> +                       spin_unlock_irqrestore(&e->pending_lock, flags);
> +
> +                       /* Clear TFR Ack if no more doorbells pending */
> +                       if (!fired_dbs)
> +                               writel_relaxed_bitfield(0x1,
> +                                                       &mhu->pbx->dbcw[channel].int_clr,
> +                                                       tfr_ack);
> +               } else {
> +                       st = readl_relaxed(&mhu->mbx->dbcw[channel].st_msk);
> +                       if (!st) {
> +                               dev_warn(dev, "Spurios IRQ on %s channel:%d\n",
> +                                        mhuv3_str[mhu->frame], channel);
> +                               continue;
> +                       }
> +                       db = __builtin_ctz(st);
> +               }
> +
> +               if (db != MHUV3_INVALID_DOORBELL) {
> +                       dev_dbg(dev, "Found %s ch[%d]/db[%d]\n",
> +                               mhuv3_str[mhu->frame], channel, db);
> +
> +                       return &mhu->mbox.chans[channel * MHUV3_STAT_BITS + db];
> +               }
> +       }
> +
> +       return ERR_PTR(-EIO);
> +}
> +
> +static int mhuv3_dbe_init(struct mhuv3 *mhu)
> +{
> +       struct mhuv3_extension *e;
> +       struct device *dev = mhu->mbox.dev;
> +
> +       if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, dbe_spt))
> +               return 0;
> +
> +       dev_dbg(dev, "%s: Initializing DBE Extension.\n", mhuv3_str[mhu->frame]);
> +
> +       e = devm_kzalloc(dev, sizeof(*e), GFP_KERNEL);
> +       if (!e)
> +               return -ENOMEM;
> +
> +       e->type = DBE_EXT;
> +       /* Note that, by the spec, the number of channels is (num_dbch + 1) */
> +       e->max_chans =
> +               readl_relaxed_bitfield(&mhu->ctrl->dbch_cfg0, num_dbch) + 1;
> +       e->mbox_of_xlate = mhuv3_dbe_mbox_of_xlate;
> +       e->combined_irq_setup = mhuv3_dbe_combined_irq_setup;
> +       e->channels_init = mhuv3_dbe_channels_init;
> +       e->chan_from_comb_irq_get = mhuv3_dbe_chan_from_comb_irq_get;
> +
> +       mhu->tot_chans += e->max_chans * MHUV3_STAT_BITS;
> +       mhu->ext[DBE_EXT] = e;
> +
> +       dev_info(dev, "%s: found %d DBE channels.\n",
> +                mhuv3_str[mhu->frame], e->max_chans);
> +
> +       return 0;
> +}
> +
> +static int mhuv3_fce_init(struct mhuv3 *mhu)
> +{
> +       struct device *dev = mhu->mbox.dev;
> +
> +       if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, fce_spt))
> +               return 0;
> +
> +       dev_dbg(dev, "%s: FCE Extension not supported by driver.\n",
> +               mhuv3_str[mhu->frame]);
> +
> +       return 0;
> +}
> +
> +static int mhuv3_fe_init(struct mhuv3 *mhu)
> +{
> +       struct device *dev = mhu->mbox.dev;
> +
> +       if (!readl_relaxed_bitfield(&mhu->ctrl->feat_spt0, fe_spt))
> +               return 0;
> +
> +       dev_dbg(dev, "%s: FE Extension not supported by driver.\n",
> +               mhuv3_str[mhu->frame]);
> +
> +       return 0;
> +}
> +
> +static mhuv3_extension_initializer mhuv3_extension_init[MAX_EXT] = {
> +       mhuv3_dbe_init,
> +       mhuv3_fce_init,
> +       mhuv3_fe_init,
> +};
> +
> +static int mhuv3_initialize_channels(struct device *dev, struct mhuv3 *mhu)
> +{
> +       int i, ret = 0;
> +       struct mbox_controller *mbox = &mhu->mbox;
> +
> +       mbox->chans = devm_kcalloc(dev, mhu->tot_chans,
> +                                  sizeof(*mbox->chans), GFP_KERNEL);
> +       if (!mbox->chans)
> +               return -ENOMEM;
> +
> +       for (i = FIRST_EXT; i < MAX_EXT && !ret; i++)
> +               if (mhu->ext[i])
> +                       ret = mhu->ext[i]->channels_init(mhu);
> +
> +       return ret;
> +}
> +
> +static struct mbox_chan *mhuv3_mbox_of_xlate(struct mbox_controller *mbox,
> +                                            const struct of_phandle_args *pa)
> +{
> +       unsigned int type, channel, param;
> +       struct mhuv3 *mhu = mhu_from_mbox(mbox);
> +
> +       if (pa->args_count != MHUV3_MBOX_CELLS)
> +               return ERR_PTR(-EINVAL);
> +
> +       type = pa->args[MHUV3_MBOX_CELL_TYPE];
> +       if (type >= MAX_EXT)
> +               return ERR_PTR(-EINVAL);
> +
> +       channel = pa->args[MHUV3_MBOX_CELL_CHWN];
> +       param = pa->args[MHUV3_MBOX_CELL_PARAM];
> +
> +       return mhu->ext[type]->mbox_of_xlate(mhu, channel, param);
> +}
> +
> +static int mhuv3_frame_init(struct mhuv3 *mhu, void __iomem *regs)
> +{
> +       int i, ret = 0;
> +       struct device *dev = mhu->mbox.dev;
> +
> +       mhu->ctrl = regs;
> +       mhu->frame = readl_relaxed_bitfield(&mhu->ctrl->blk_id, blk_id);
> +       if (mhu->frame > MBX_FRAME) {
> +               dev_err(dev, "Invalid Frame type- %d\n", mhu->frame);
> +               return -EINVAL;
> +       }
> +
> +       mhu->major = readl_relaxed_bitfield(&mhu->ctrl->aidr, arch_major_rev);
> +       mhu->minor = readl_relaxed_bitfield(&mhu->ctrl->aidr, arch_minor_rev);
> +       if (mhu->major != MHUV3_MAJOR_VERSION) {
> +               dev_warn(dev, "Unsupported MHU %s block - major:%d  minor:%d\n",
> +                        mhuv3_str[mhu->frame], mhu->major, mhu->minor);
> +               return -EINVAL;
> +       }
> +       mhu->auto_op_full = !!readl_relaxed_bitfield(&mhu->ctrl->feat_spt1,
> +                                                    auto_op_spt);
> +       /* Request the PBX/MBX to remain operational */
> +       if (mhu->auto_op_full)
> +               writel_relaxed_bitfield(0x1, &mhu->ctrl->ctrl, op_req);
> +
> +       dev_dbg(dev, "Found MHU %s block - major:%d  minor:%d\n",
> +               mhuv3_str[mhu->frame], mhu->major, mhu->minor);
> +
> +       if (mhu->frame == PBX_FRAME)
> +               mhu->pbx = regs;
> +       else
> +               mhu->mbx = regs;
> +
> +       for (i = FIRST_EXT; i < MAX_EXT && !ret; i++)
> +               ret = mhuv3_extension_init[i](mhu);
> +
> +       return ret;
> +}
> +
> +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.

> +                       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.


> +                       void *data = NULL;
> +                       struct mhuv3_mbox_chan_priv *priv = chan->con_priv;
> +
> +                       found++;
> +                       /* Read and acknowledge optional in-band LE data first. */
> +                       if (priv->ops->read_data)
> +                               data = priv->ops->read_data(mhu, chan);
> +
> +                       if (chan->cl && !IS_ERR(data)) {
> +                               mbox_chan_received_data(chan, data);
> +                               ret = IRQ_HANDLED;
> +                       } else if (!chan->cl) {
> +                               dev_warn(dev,
> +                                        "RX Data on UNBOUND channel (%u)\n",
> +                                        priv->ch_idx);
> +                       } else {
> +                               dev_err(dev, "Failed to read data: %lu\n",
> +                                       PTR_ERR(data));
> +                       }
> +
> +                       if (!IS_ERR(data))
> +                               kfree(data);
> +
> +                       /*
> +                        * Acknowledge transfer after any possible optional
> +                        * out-of-band data has also been retrieved via
> +                        * mbox_chan_received_data().
> +                        */
> +                       if (priv->ops->rx_complete)
> +                               priv->ops->rx_complete(mhu, chan);
> +               }
> +       }
> +
> +       if (!found)
> +               dev_warn_once(dev, "Failed to find channel for the RX interrupt\n");
> +
> +       return ret;
> +}
> +
> +static int mhuv3_setup_pbx(struct mhuv3 *mhu)
> +{
> +       struct device *dev = mhu->mbox.dev;
> +
> +       mhu->mbox.ops = &mhuv3_sender_ops;
> +
> +       if (mhu->cmb_irq > 0) {
> +               int ret;
> +
> +               ret = devm_request_threaded_irq(dev, mhu->cmb_irq, NULL,
> +                                               mhuv3_pbx_comb_interrupt,
> +                                               IRQF_ONESHOT, "mhuv3-pbx", mhu);
> +               if (!ret) {
> +                       int i;
> +
> +                       mhu->mbox.txdone_irq = true;
> +                       mhu->mbox.txdone_poll = false;
> +
> +                       for (i = FIRST_EXT; i < MAX_EXT; i++)
> +                               if (mhu->ext[i])
> +                                       mhu->ext[i]->combined_irq_setup(mhu);
> +
> +                       dev_dbg(dev, "MHUv3 PBX IRQs initialized.\n");
> +
> +                       return 0;
> +               }
> +
> +               dev_err(dev, "Failed to request PBX IRQ - ret:%d", ret);
> +       }
> +
> +       dev_info(dev, "Using PBX in Tx polling mode.\n");
> +       mhu->mbox.txdone_irq = false;
> +       mhu->mbox.txdone_poll = true;
> +       mhu->mbox.txpoll_period = 1;
> +
> +       return 0;
> +}
> +
> +static int mhuv3_setup_mbx(struct mhuv3 *mhu)
> +{
> +       int ret, i;
> +       struct device *dev = mhu->mbox.dev;
> +
> +       mhu->mbox.ops = &mhuv3_receiver_ops;
> +
> +       if (mhu->cmb_irq <= 0) {
> +               dev_err(dev, "Missing MBX combined IRQ !\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = devm_request_threaded_irq(dev, mhu->cmb_irq, NULL,
> +                                       mhuv3_mbx_comb_interrupt, IRQF_ONESHOT,
> +                                       "mhuv3-mbx", mhu);
> +       if (ret) {
> +               dev_err(dev, "Failed to request MBX IRQ - ret:%d\n", ret);
> +               return ret;
> +       }
> +
> +       for (i = FIRST_EXT; i < MAX_EXT; i++)
> +               if (mhu->ext[i])
> +                       mhu->ext[i]->combined_irq_setup(mhu);
> +
> +       dev_dbg(dev, "MHUv3 MBX IRQs initialized.\n");
> +
> +       return ret;
> +}
> +
> +static int mhuv3_irqs_init(struct mhuv3 *mhu, struct platform_device *pdev)
> +{
> +       int ret;
> +
> +       dev_dbg(mhu->mbox.dev, "Initializing %s block.\n", mhuv3_str[mhu->frame]);
> +
> +       if (mhu->frame == PBX_FRAME) {
> +               mhu->cmb_irq = platform_get_irq_byname_optional(pdev, "combined");
> +               ret = mhuv3_setup_pbx(mhu);
> +       } else {
> +               mhu->cmb_irq = platform_get_irq_byname(pdev, "combined");
> +               ret = mhuv3_setup_mbx(mhu);
> +       }
> +
> +       return ret;
> +}
> +
> +static int mhuv3_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct mhuv3 *mhu;
> +       void __iomem *regs;
> +       struct device *dev = &pdev->dev;
> +
> +       mhu = devm_kzalloc(dev, sizeof(*mhu), GFP_KERNEL);
> +       if (!mhu)
> +               return -ENOMEM;
> +
> +       regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(regs))
> +               return PTR_ERR(regs);
> +
> +       mhu->mbox.dev = dev;
> +       ret = mhuv3_frame_init(mhu, regs);
> +       if (ret)
> +               return ret;
> +
> +       ret = mhuv3_irqs_init(mhu, pdev);
> +       if (ret)
> +               return ret;
> +
> +       mhu->mbox.of_xlate = mhuv3_mbox_of_xlate;
> +       ret = mhuv3_initialize_channels(dev, mhu);
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_mbox_controller_register(dev, &mhu->mbox);
> +       if (ret)
> +               dev_err(dev, "failed to register ARM MHUv3 driver %d\n", ret);
> +
> +       platform_set_drvdata(pdev, mhu);
> +
> +       return ret;
> +}
> +
> +static int mhuv3_remove(struct platform_device *pdev)
> +{
> +       struct mhuv3 *mhu = platform_get_drvdata(pdev);
> +
> +       if (mhu->auto_op_full)
> +               writel_relaxed_bitfield(0x0, &mhu->ctrl->ctrl, op_req);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id mhuv3_of_match[] = {
> +       { .compatible = "arm,mhuv3", .data = NULL },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, mhuv3_of_match);
> +
> +static struct platform_driver mhuv3_driver = {
> +       .driver = {
> +               .name = "arm-mhuv3-mailbox",
> +               .of_match_table = mhuv3_of_match,
> +       },
> +       .probe = mhuv3_probe,
> +       .remove = mhuv3_remove,
> +};
> +module_platform_driver(mhuv3_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ARM MHUv3 Driver");
> +MODULE_AUTHOR("Cristian Marussi <cristian.marussi@arm.com>");
> --
> 2.34.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: [RFC PATCH v2 4/5] clk: meson: a1: add the audio clock controller driver
From: Jan Dakinevich @ 2024-04-08  1:07 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
	Martin Blumenstingl, Philipp Zabel, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1j34s3iy9c.fsf@starbuckisacylon.baylibre.com>



On 4/2/24 18:11, Jerome Brunet wrote:
> 
> On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> 
>> This controller provides clocks and reset functionality for audio
>> peripherals on Amlogic A1 SoC family.
>>
>> The driver is almost identical to 'axg-audio', however it would be better
>> to keep it separate due to following reasons:
>>
>>  - significant amount of bits has another definition. I will bring there
>>    a mess of new defines with A1_ suffixes.
>>
>>  - registers of this controller are located in two separate regions. It
>>    will give a lot of complications for 'axg-audio' to support this.
>>
>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> ---
>>  drivers/clk/meson/Kconfig    |  13 +
>>  drivers/clk/meson/Makefile   |   1 +
>>  drivers/clk/meson/a1-audio.c | 624 +++++++++++++++++++++++++++++++++++
>>  drivers/clk/meson/a1-audio.h |  45 +++
>>  4 files changed, 683 insertions(+)
>>  create mode 100644 drivers/clk/meson/a1-audio.c
>>  create mode 100644 drivers/clk/meson/a1-audio.h
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index d6a2fa5f7e88..80c4a18c83d2 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>  	  device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>  	  controller to work.
>>  
>> +config COMMON_CLK_A1_AUDIO
>> +	tristate "Amlogic A1 SoC Audio clock controller support"
>> +	depends on ARM64
>> +	select COMMON_CLK_MESON_REGMAP
>> +	select COMMON_CLK_MESON_CLKC_UTILS
>> +	select COMMON_CLK_MESON_PHASE
>> +	select COMMON_CLK_MESON_SCLK_DIV
>> +	select COMMON_CLK_MESON_AUDIO_RSTC
>> +	help
>> +	  Support for the Audio clock controller on Amlogic A113L based
>> +	  device, A1 SoC Family. Say Y if you want A1 Audio clock controller
>> +	  to work.
>> +
>>  config COMMON_CLK_G12A
>>  	tristate "G12 and SM1 SoC clock controllers support"
>>  	depends on ARM64
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 88d94921a4dc..4968fc7ad555 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>  obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>  obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>  obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>>  obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>  obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
>> new file mode 100644
>> index 000000000000..bd2b6dde75d4
>> --- /dev/null
>> +++ b/drivers/clk/meson/a1-audio.c
>> @@ -0,0 +1,624 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +/*
>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> + *
>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/init.h>
>> +#include <linux/of_device.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/slab.h>
>> +
>> +#include "meson-clkc-utils.h"
>> +#include "meson-audio-rstc.h"
>> +#include "meson-audio.h"
>> +#include "clk-regmap.h"
>> +#include "clk-phase.h"
>> +#include "sclk-div.h"
>> +#include "a1-audio.h"
>> +
>> +static const struct clk_parent_data a1_pclk_pdata[] = {
>> +	{ .fw_name = "pclk" },
>> +};
> 
> Shouldn't you go through AUD2_CLKID_AUDIOTOP instead ?
> 
>> +
>> +#define AUD_PCLK_GATE(_name, _reg, _bit) {				\
>> +	.data = &(struct clk_regmap_gate_data){				\
>> +		.offset = (_reg),					\
>> +		.bit_idx = (_bit),					\
>> +	},								\
>> +	.hw.init = &(struct clk_init_data) {				\
>> +		.name = "aud_"#_name,					\
>> +		.ops = &clk_regmap_gate_ops,				\
>> +		.parent_data = a1_pclk_pdata,				\
>> +		.num_parents = 1,					\
>> +	},								\
>> +}
>> +
>> +struct clk_regmap aud_ddr_arb =
>> +	AUD_PCLK_GATE(ddr_arb, AUDIO_CLK_GATE_EN0, 0);
>> +struct clk_regmap aud_tdmin_a =
>> +	AUD_PCLK_GATE(tdmin_a, AUDIO_CLK_GATE_EN0, 1);
>> +struct clk_regmap aud_tdmin_b =
>> +	AUD_PCLK_GATE(tdmin_b, AUDIO_CLK_GATE_EN0, 2);
>> +struct clk_regmap aud_tdmin_lb =
>> +	AUD_PCLK_GATE(tdmin_lb, AUDIO_CLK_GATE_EN0, 3);
>> +struct clk_regmap aud_loopback =
>> +	AUD_PCLK_GATE(loopback, AUDIO_CLK_GATE_EN0, 4);
>> +struct clk_regmap aud_tdmout_a =
>> +	AUD_PCLK_GATE(tdmout_a, AUDIO_CLK_GATE_EN0, 5);
>> +struct clk_regmap aud_tdmout_b =
>> +	AUD_PCLK_GATE(tdmout_b, AUDIO_CLK_GATE_EN0, 6);
>> +struct clk_regmap aud_frddr_a =
>> +	AUD_PCLK_GATE(frddr_a, AUDIO_CLK_GATE_EN0, 7);
>> +struct clk_regmap aud_frddr_b =
>> +	AUD_PCLK_GATE(frddr_b, AUDIO_CLK_GATE_EN0, 8);
>> +struct clk_regmap aud_toddr_a =
>> +	AUD_PCLK_GATE(toddr_a, AUDIO_CLK_GATE_EN0, 9);
>> +struct clk_regmap aud_toddr_b =
>> +	AUD_PCLK_GATE(toddr_b, AUDIO_CLK_GATE_EN0, 10);
>> +struct clk_regmap aud_spdifin =
>> +	AUD_PCLK_GATE(spdifin, AUDIO_CLK_GATE_EN0, 11);
>> +struct clk_regmap aud_resample =
>> +	AUD_PCLK_GATE(resample, AUDIO_CLK_GATE_EN0, 12);
>> +struct clk_regmap aud_eqdrc =
>> +	AUD_PCLK_GATE(eqdrc, AUDIO_CLK_GATE_EN0, 13);
>> +struct clk_regmap aud_audiolocker =
>> +	AUD_PCLK_GATE(audiolocker, AUDIO_CLK_GATE_EN0, 14);
>> +
>> +struct clk_regmap aud2_ddr_arb =
>> +	AUD_PCLK_GATE(2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0);
>> +struct clk_regmap aud2_pdm =
>> +	AUD_PCLK_GATE(2_pdm, AUDIO2_CLK_GATE_EN0, 1);
>> +struct clk_regmap aud2_tdmin_vad =
>> +	AUD_PCLK_GATE(2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2);
>> +struct clk_regmap aud2_toddr_vad =
>> +	AUD_PCLK_GATE(2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3);
>> +struct clk_regmap aud2_vad =
>> +	AUD_PCLK_GATE(2_vad, AUDIO2_CLK_GATE_EN0, 4);
>> +struct clk_regmap aud2_audiotop =
>> +	AUD_PCLK_GATE(2_audiotop, AUDIO2_CLK_GATE_EN0, 7);
>> +
>> +static const struct clk_parent_data a1_mst_pdata[] = {
>> +	{ .fw_name = "dds_in" },
>> +	{ .fw_name = "fclk_div2" },
>> +	{ .fw_name = "fclk_div3" },
>> +	{ .fw_name = "hifi_pll" },
>> +	{ .fw_name = "xtal" },
>> +};
>> +
>> +#define AUD_MST_MCLK_MUX(_name, _reg)					\
>> +	AUD_MUX(_name##_sel, _reg, 0x7, 24, CLK_MUX_ROUND_CLOSEST,	\
>> +		a1_mst_pdata, 0)
>> +#define AUD_MST_MCLK_DIV(_name, _reg)					\
>> +	AUD_DIV(_name##_div, _reg, 0, 16, CLK_DIVIDER_ROUND_CLOSEST,	\
>> +		aud_##_name##_sel, CLK_SET_RATE_PARENT)
>> +#define AUD_MST_MCLK_GATE(_name, _reg)					\
>> +	AUD_GATE(_name, _reg, 31, aud_##_name##_div,			\
>> +		 CLK_SET_RATE_PARENT)
>> +
>> +struct clk_regmap aud_mst_a_mclk_mux =
>> +	AUD_MST_MCLK_MUX(mst_a_mclk, AUDIO_MCLK_A_CTRL);
>> +struct clk_regmap aud_mst_a_mclk_div =
>> +	AUD_MST_MCLK_DIV(mst_a_mclk, AUDIO_MCLK_A_CTRL);
>> +struct clk_regmap aud_mst_a_mclk =
>> +	AUD_MST_MCLK_GATE(mst_a_mclk, AUDIO_MCLK_A_CTRL);
>> +
>> +struct clk_regmap aud_mst_b_mclk_mux =
>> +	AUD_MST_MCLK_MUX(mst_b_mclk, AUDIO_MCLK_B_CTRL);
>> +struct clk_regmap aud_mst_b_mclk_div =
>> +	AUD_MST_MCLK_DIV(mst_b_mclk, AUDIO_MCLK_B_CTRL);
>> +struct clk_regmap aud_mst_b_mclk =
>> +	AUD_MST_MCLK_GATE(mst_b_mclk, AUDIO_MCLK_B_CTRL);
>> +
>> +struct clk_regmap aud_mst_c_mclk_mux =
>> +	AUD_MST_MCLK_MUX(mst_c_mclk, AUDIO_MCLK_C_CTRL);
>> +struct clk_regmap aud_mst_c_mclk_div =
>> +	AUD_MST_MCLK_DIV(mst_c_mclk, AUDIO_MCLK_C_CTRL);
>> +struct clk_regmap aud_mst_c_mclk =
>> +	AUD_MST_MCLK_GATE(mst_c_mclk, AUDIO_MCLK_C_CTRL);
>> +
>> +struct clk_regmap aud_mst_d_mclk_mux =
>> +	AUD_MST_MCLK_MUX(mst_d_mclk, AUDIO_MCLK_D_CTRL);
>> +struct clk_regmap aud_mst_d_mclk_div =
>> +	AUD_MST_MCLK_DIV(mst_d_mclk, AUDIO_MCLK_D_CTRL);
>> +struct clk_regmap aud_mst_d_mclk =
>> +	AUD_MST_MCLK_GATE(mst_d_mclk, AUDIO_MCLK_D_CTRL);
>> +
>> +struct clk_regmap aud_spdifin_clk_mux =
>> +	AUD_MST_MCLK_MUX(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>> +struct clk_regmap aud_spdifin_clk_div =
>> +	AUD_MST_MCLK_DIV(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>> +struct clk_regmap aud_spdifin_clk =
>> +	AUD_MST_MCLK_GATE(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>> +
>> +struct clk_regmap aud_eqdrc_clk_mux =
>> +	AUD_MST_MCLK_MUX(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>> +struct clk_regmap aud_eqdrc_clk_div =
>> +	AUD_MST_MCLK_DIV(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>> +struct clk_regmap aud_eqdrc_clk =
>> +	AUD_MST_MCLK_GATE(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>> +
>> +struct clk_regmap aud_resample_clk_mux =
>> +	AUD_MUX(resample_clk_sel, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
>> +		CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_resample_clk_div =
>> +	AUD_DIV(resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
>> +		CLK_DIVIDER_ROUND_CLOSEST, resample_clk_sel, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_resample_clk =
>> +	AUD_GATE(resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
>> +		 resample_clk_div, CLK_SET_RATE_PARENT);
>> +
>> +struct clk_regmap aud_locker_in_clk_mux =
>> +	AUD_MUX(locker_in_clk_sel, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
>> +		CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_locker_in_clk_div =
>> +	AUD_DIV(locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
>> +		CLK_DIVIDER_ROUND_CLOSEST, locker_in_clk_sel, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_locker_in_clk =
>> +	AUD_GATE(locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
>> +		 locker_in_clk_div, CLK_SET_RATE_PARENT);
>> +
>> +struct clk_regmap aud_locker_out_clk_mux =
>> +	AUD_MUX(locker_out_clk_sel, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
>> +		CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_locker_out_clk_div =
>> +	AUD_DIV(locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
>> +		CLK_DIVIDER_ROUND_CLOSEST, locker_out_clk_sel, CLK_SET_RATE_PARENT);
>> +struct clk_regmap aud_locker_out_clk =
>> +	AUD_GATE(locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
>> +		 locker_out_clk_div, CLK_SET_RATE_PARENT);
>> +
>> +struct clk_regmap aud2_vad_mclk_mux =
>> +	AUD_MST_MCLK_MUX(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>> +struct clk_regmap aud2_vad_mclk_div =
>> +	AUD_MST_MCLK_DIV(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>> +struct clk_regmap aud2_vad_mclk =
>> +	AUD_MST_MCLK_GATE(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>> +
>> +struct clk_regmap aud2_vad_clk_mux =
>> +	AUD_MST_MCLK_MUX(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>> +struct clk_regmap aud2_vad_clk_div =
>> +	AUD_MST_MCLK_DIV(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>> +struct clk_regmap aud2_vad_clk =
>> +	AUD_MST_MCLK_GATE(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>> +
>> +struct clk_regmap aud2_pdm_dclk_mux =
>> +	AUD_MST_MCLK_MUX(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>> +struct clk_regmap aud2_pdm_dclk_div =
>> +	AUD_MST_MCLK_DIV(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>> +struct clk_regmap aud2_pdm_dclk =
>> +	AUD_MST_MCLK_GATE(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>> +
>> +struct clk_regmap aud2_pdm_sysclk_mux =
>> +	AUD_MST_MCLK_MUX(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>> +struct clk_regmap aud2_pdm_sysclk_div =
>> +	AUD_MST_MCLK_DIV(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>> +struct clk_regmap aud2_pdm_sysclk =
>> +	AUD_MST_MCLK_GATE(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>> +
>> +#define AUD_MST_SCLK_PRE_EN(_name, _reg, _pname)			\
>> +	AUD_GATE(_name##_pre_en, _reg, 31,				\
>> +		 aud_##_pname, 0)
>> +#define AUD_MST_SCLK_DIV(_name, _reg)					\
>> +	AUD_SCLK_DIV(_name##_div, _reg, 20, 10, 0, 0,			\
>> +		     aud_##_name##_pre_en, CLK_SET_RATE_PARENT)
>> +#define AUD_MST_SCLK_POST_EN(_name, _reg)				\
>> +	AUD_GATE(_name##_post_en, _reg, 30,				\
>> +		 aud_##_name##_div, CLK_SET_RATE_PARENT)
>> +#define AUD_MST_SCLK(_name, _reg)					\
>> +	AUD_TRIPHASE(_name, _reg, 1, 0, 2, 4,				\
>> +		     aud_##_name##_post_en, CLK_SET_RATE_PARENT)
>> +
>> +struct clk_regmap aud_mst_a_sclk_pre_en =
>> +	AUD_MST_SCLK_PRE_EN(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, mst_a_mclk);
>> +struct clk_regmap aud_mst_a_sclk_div =
>> +	AUD_MST_SCLK_DIV(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_a_sclk_post_en =
>> +	AUD_MST_SCLK_POST_EN(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_a_sclk =
>> +	AUD_MST_SCLK(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_b_sclk_pre_en =
>> +	AUD_MST_SCLK_PRE_EN(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, mst_b_mclk);
>> +struct clk_regmap aud_mst_b_sclk_div =
>> +	AUD_MST_SCLK_DIV(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_b_sclk_post_en =
>> +	AUD_MST_SCLK_POST_EN(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_b_sclk =
>> +	AUD_MST_SCLK(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_c_sclk_pre_en =
>> +	AUD_MST_SCLK_PRE_EN(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, mst_c_mclk);
>> +struct clk_regmap aud_mst_c_sclk_div =
>> +	AUD_MST_SCLK_DIV(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_c_sclk_post_en =
>> +	AUD_MST_SCLK_POST_EN(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_c_sclk =
>> +	AUD_MST_SCLK(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_d_sclk_pre_en =
>> +	AUD_MST_SCLK_PRE_EN(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, mst_d_mclk);
>> +struct clk_regmap aud_mst_d_sclk_div =
>> +	AUD_MST_SCLK_DIV(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_d_sclk_post_en =
>> +	AUD_MST_SCLK_POST_EN(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0);
>> +struct clk_regmap aud_mst_d_sclk =
>> +	AUD_MST_SCLK(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL1);
>> +
>> +#define AUD_MST_LRCLK_DIV(_name, _reg, _pname)				\
>> +	AUD_SCLK_DIV(_name##_div, _reg, 0, 10, 10, 10,			\
>> +		     aud_##_pname, 0)
>> +#define AUD_MST_LRCLK(_name, _reg)					\
>> +	AUD_TRIPHASE(_name, _reg, 1, 1, 3, 5,				\
>> +		     aud_##_name##_div, CLK_SET_RATE_PARENT)
>> +
>> +struct clk_regmap aud_mst_a_lrclk_div =
>> +	AUD_MST_LRCLK_DIV(mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, mst_a_sclk_post_en);
>> +struct clk_regmap aud_mst_a_lrclk =
>> +	AUD_MST_LRCLK(mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_b_lrclk_div =
>> +	AUD_MST_LRCLK_DIV(mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, mst_b_sclk_post_en);
>> +struct clk_regmap aud_mst_b_lrclk =
>> +	AUD_MST_LRCLK(mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_c_lrclk_div =
>> +	AUD_MST_LRCLK_DIV(mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, mst_c_sclk_post_en);
>> +struct clk_regmap aud_mst_c_lrclk =
>> +	AUD_MST_LRCLK(mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL1);
>> +
>> +struct clk_regmap aud_mst_d_lrclk_div =
>> +	AUD_MST_LRCLK_DIV(mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, mst_d_sclk_post_en);
>> +struct clk_regmap aud_mst_d_lrclk =
>> +	AUD_MST_LRCLK(mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL1);
>> +
>> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
>> +	{ .hw = &aud_mst_a_sclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_b_sclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_c_sclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_d_sclk.hw, .index = -1 },
>> +	{ .fw_name = "slv_sclk0" },
>> +	{ .fw_name = "slv_sclk1" },
>> +	{ .fw_name = "slv_sclk2" },
>> +	{ .fw_name = "slv_sclk3" },
>> +	{ .fw_name = "slv_sclk4" },
>> +	{ .fw_name = "slv_sclk5" },
>> +	{ .fw_name = "slv_sclk6" },
>> +	{ .fw_name = "slv_sclk7" },
>> +	{ .fw_name = "slv_sclk8" },
>> +	{ .fw_name = "slv_sclk9" },
>> +};
>> +
>> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
>> +	{ .hw = &aud_mst_a_lrclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_b_lrclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_c_lrclk.hw, .index = -1 },
>> +	{ .hw = &aud_mst_d_lrclk.hw, .index = -1 },
>> +	{ .fw_name = "slv_lrclk0" },
>> +	{ .fw_name = "slv_lrclk1" },
>> +	{ .fw_name = "slv_lrclk2" },
>> +	{ .fw_name = "slv_lrclk3" },
>> +	{ .fw_name = "slv_lrclk4" },
>> +	{ .fw_name = "slv_lrclk5" },
>> +	{ .fw_name = "slv_lrclk6" },
>> +	{ .fw_name = "slv_lrclk7" },
>> +	{ .fw_name = "slv_lrclk8" },
>> +	{ .fw_name = "slv_lrclk9" },
>> +};
>> +
>> +#define AUD_TDM_SCLK_MUX(_name, _reg)					\
>> +	AUD_MUX(_name##_sel, _reg, 0xf, 24,				\
>> +		CLK_MUX_ROUND_CLOSEST, a1_mst_sclk_pdata, 0)
>> +#define AUD_TDM_SCLK_PRE_EN(_name, _reg)				\
>> +	AUD_GATE(_name##_pre_en, _reg, 31,				\
>> +		 aud_##_name##_sel, CLK_SET_RATE_PARENT)
>> +#define AUD_TDM_SCLK_POST_EN(_name, _reg)				\
>> +	AUD_GATE(_name##_post_en, _reg, 30,				\
>> +		 aud_##_name##_pre_en, CLK_SET_RATE_PARENT)
>> +#define AUD_TDM_SCLK_WS(_name, _reg)					\
>> +	AUD_SCLK_WS(_name, _reg, 1, 29, 28,				\
>> +		    aud_##_name##_post_en,				\
>> +		    CLK_DUTY_CYCLE_PARENT | CLK_SET_RATE_PARENT)
>> +
>> +#define AUD_TDM_LRLCK(_name, _reg)					\
>> +	AUD_MUX(_name, _reg, 0xf, 20,					\
>> +		CLK_MUX_ROUND_CLOSEST, a1_mst_lrclk_pdata,		\
>> +		CLK_SET_RATE_PARENT)
>> +
>> +struct clk_regmap aud_tdmin_a_sclk_mux =
>> +	AUD_TDM_SCLK_MUX(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>> +struct clk_regmap aud_tdmin_a_sclk_pre_en =
>> +	AUD_TDM_SCLK_PRE_EN(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>> +struct clk_regmap aud_tdmin_a_sclk_post_en =
>> +	AUD_TDM_SCLK_POST_EN(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>> +struct clk_regmap aud_tdmin_a_sclk =
>> +	AUD_TDM_SCLK_WS(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>> +struct clk_regmap aud_tdmin_a_lrclk =
>> +	AUD_TDM_LRLCK(tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
>> +
>> +struct clk_regmap aud_tdmin_b_sclk_mux =
>> +	AUD_TDM_SCLK_MUX(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>> +struct clk_regmap aud_tdmin_b_sclk_pre_en =
>> +	AUD_TDM_SCLK_PRE_EN(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>> +struct clk_regmap aud_tdmin_b_sclk_post_en =
>> +	AUD_TDM_SCLK_POST_EN(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>> +struct clk_regmap aud_tdmin_b_sclk =
>> +	AUD_TDM_SCLK_WS(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>> +struct clk_regmap aud_tdmin_b_lrclk =
>> +	AUD_TDM_LRLCK(tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
>> +
>> +struct clk_regmap aud_tdmin_lb_sclk_mux =
>> +	AUD_TDM_SCLK_MUX(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>> +struct clk_regmap aud_tdmin_lb_sclk_pre_en =
>> +	AUD_TDM_SCLK_PRE_EN(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>> +struct clk_regmap aud_tdmin_lb_sclk_post_en =
>> +	AUD_TDM_SCLK_POST_EN(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>> +struct clk_regmap aud_tdmin_lb_sclk =
>> +	AUD_TDM_SCLK_WS(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>> +struct clk_regmap aud_tdmin_lb_lrclk =
>> +	AUD_TDM_LRLCK(tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
>> +
>> +struct clk_regmap aud_tdmout_a_sclk_mux =
>> +	AUD_TDM_SCLK_MUX(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>> +struct clk_regmap aud_tdmout_a_sclk_pre_en =
>> +	AUD_TDM_SCLK_PRE_EN(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>> +struct clk_regmap aud_tdmout_a_sclk_post_en =
>> +	AUD_TDM_SCLK_POST_EN(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>> +struct clk_regmap aud_tdmout_a_sclk =
>> +	AUD_TDM_SCLK_WS(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>> +struct clk_regmap aud_tdmout_a_lrclk =
>> +	AUD_TDM_LRLCK(tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
>> +
>> +struct clk_regmap aud_tdmout_b_sclk_mux =
>> +	AUD_TDM_SCLK_MUX(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>> +struct clk_regmap aud_tdmout_b_sclk_pre_en =
>> +	AUD_TDM_SCLK_PRE_EN(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>> +struct clk_regmap aud_tdmout_b_sclk_post_en =
>> +	AUD_TDM_SCLK_POST_EN(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>> +struct clk_regmap aud_tdmout_b_sclk =
>> +	AUD_TDM_SCLK_WS(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>> +struct clk_regmap aud_tdmout_b_lrclk =
>> +	AUD_TDM_LRLCK(tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
>> +
>> +static struct clk_hw *a1_audio_hw_clks[] = {
>> +	[AUD_CLKID_DDR_ARB]		= &aud_ddr_arb.hw,
>> +	[AUD_CLKID_TDMIN_A]		= &aud_tdmin_a.hw,
>> +	[AUD_CLKID_TDMIN_B]		= &aud_tdmin_b.hw,
>> +	[AUD_CLKID_TDMIN_LB]		= &aud_tdmin_lb.hw,
>> +	[AUD_CLKID_LOOPBACK]		= &aud_loopback.hw,
>> +	[AUD_CLKID_TDMOUT_A]		= &aud_tdmout_a.hw,
>> +	[AUD_CLKID_TDMOUT_B]		= &aud_tdmout_b.hw,
>> +	[AUD_CLKID_FRDDR_A]		= &aud_frddr_a.hw,
>> +	[AUD_CLKID_FRDDR_B]		= &aud_frddr_b.hw,
>> +	[AUD_CLKID_TODDR_A]		= &aud_toddr_a.hw,
>> +	[AUD_CLKID_TODDR_B]		= &aud_toddr_b.hw,
>> +	[AUD_CLKID_SPDIFIN]		= &aud_spdifin.hw,
>> +	[AUD_CLKID_RESAMPLE]		= &aud_resample.hw,
>> +	[AUD_CLKID_EQDRC]		= &aud_eqdrc.hw,
>> +	[AUD_CLKID_LOCKER]		= &aud_audiolocker.hw,
>> +	[AUD_CLKID_MST_A_MCLK_SEL]	= &aud_mst_a_mclk_mux.hw,
>> +	[AUD_CLKID_MST_A_MCLK_DIV]	= &aud_mst_a_mclk_div.hw,
>> +	[AUD_CLKID_MST_A_MCLK]		= &aud_mst_a_mclk.hw,
>> +	[AUD_CLKID_MST_B_MCLK_SEL]	= &aud_mst_b_mclk_mux.hw,
>> +	[AUD_CLKID_MST_B_MCLK_DIV]	= &aud_mst_b_mclk_div.hw,
>> +	[AUD_CLKID_MST_B_MCLK]		= &aud_mst_b_mclk.hw,
>> +	[AUD_CLKID_MST_C_MCLK_SEL]	= &aud_mst_c_mclk_mux.hw,
>> +	[AUD_CLKID_MST_C_MCLK_DIV]	= &aud_mst_c_mclk_div.hw,
>> +	[AUD_CLKID_MST_C_MCLK]		= &aud_mst_c_mclk.hw,
>> +	[AUD_CLKID_MST_D_MCLK_SEL]	= &aud_mst_d_mclk_mux.hw,
>> +	[AUD_CLKID_MST_D_MCLK_DIV]	= &aud_mst_d_mclk_div.hw,
>> +	[AUD_CLKID_MST_D_MCLK]		= &aud_mst_d_mclk.hw,
>> +	[AUD_CLKID_RESAMPLE_CLK_SEL]	= &aud_resample_clk_mux.hw,
>> +	[AUD_CLKID_RESAMPLE_CLK_DIV]	= &aud_resample_clk_div.hw,
>> +	[AUD_CLKID_RESAMPLE_CLK]	= &aud_resample_clk.hw,
>> +	[AUD_CLKID_LOCKER_IN_CLK_SEL]	= &aud_locker_in_clk_mux.hw,
>> +	[AUD_CLKID_LOCKER_IN_CLK_DIV]	= &aud_locker_in_clk_div.hw,
>> +	[AUD_CLKID_LOCKER_IN_CLK]	= &aud_locker_in_clk.hw,
>> +	[AUD_CLKID_LOCKER_OUT_CLK_SEL]	= &aud_locker_out_clk_mux.hw,
>> +	[AUD_CLKID_LOCKER_OUT_CLK_DIV]	= &aud_locker_out_clk_div.hw,
>> +	[AUD_CLKID_LOCKER_OUT_CLK]	= &aud_locker_out_clk.hw,
>> +	[AUD_CLKID_SPDIFIN_CLK_SEL]	= &aud_spdifin_clk_mux.hw,
>> +	[AUD_CLKID_SPDIFIN_CLK_DIV]	= &aud_spdifin_clk_div.hw,
>> +	[AUD_CLKID_SPDIFIN_CLK]		= &aud_spdifin_clk.hw,
>> +	[AUD_CLKID_EQDRC_CLK_SEL]	= &aud_eqdrc_clk_mux.hw,
>> +	[AUD_CLKID_EQDRC_CLK_DIV]	= &aud_eqdrc_clk_div.hw,
>> +	[AUD_CLKID_EQDRC_CLK]		= &aud_eqdrc_clk.hw,
>> +	[AUD_CLKID_MST_A_SCLK_PRE_EN]	= &aud_mst_a_sclk_pre_en.hw,
>> +	[AUD_CLKID_MST_A_SCLK_DIV]	= &aud_mst_a_sclk_div.hw,
>> +	[AUD_CLKID_MST_A_SCLK_POST_EN]	= &aud_mst_a_sclk_post_en.hw,
>> +	[AUD_CLKID_MST_A_SCLK]		= &aud_mst_a_sclk.hw,
>> +	[AUD_CLKID_MST_B_SCLK_PRE_EN]	= &aud_mst_b_sclk_pre_en.hw,
>> +	[AUD_CLKID_MST_B_SCLK_DIV]	= &aud_mst_b_sclk_div.hw,
>> +	[AUD_CLKID_MST_B_SCLK_POST_EN]	= &aud_mst_b_sclk_post_en.hw,
>> +	[AUD_CLKID_MST_B_SCLK]		= &aud_mst_b_sclk.hw,
>> +	[AUD_CLKID_MST_C_SCLK_PRE_EN]	= &aud_mst_c_sclk_pre_en.hw,
>> +	[AUD_CLKID_MST_C_SCLK_DIV]	= &aud_mst_c_sclk_div.hw,
>> +	[AUD_CLKID_MST_C_SCLK_POST_EN]	= &aud_mst_c_sclk_post_en.hw,
>> +	[AUD_CLKID_MST_C_SCLK]		= &aud_mst_c_sclk.hw,
>> +	[AUD_CLKID_MST_D_SCLK_PRE_EN]	= &aud_mst_d_sclk_pre_en.hw,
>> +	[AUD_CLKID_MST_D_SCLK_DIV]	= &aud_mst_d_sclk_div.hw,
>> +	[AUD_CLKID_MST_D_SCLK_POST_EN]	= &aud_mst_d_sclk_post_en.hw,
>> +	[AUD_CLKID_MST_D_SCLK]		= &aud_mst_d_sclk.hw,
>> +	[AUD_CLKID_MST_A_LRCLK_DIV]	= &aud_mst_a_lrclk_div.hw,
>> +	[AUD_CLKID_MST_A_LRCLK]		= &aud_mst_a_lrclk.hw,
>> +	[AUD_CLKID_MST_B_LRCLK_DIV]	= &aud_mst_b_lrclk_div.hw,
>> +	[AUD_CLKID_MST_B_LRCLK]		= &aud_mst_b_lrclk.hw,
>> +	[AUD_CLKID_MST_C_LRCLK_DIV]	= &aud_mst_c_lrclk_div.hw,
>> +	[AUD_CLKID_MST_C_LRCLK]		= &aud_mst_c_lrclk.hw,
>> +	[AUD_CLKID_MST_D_LRCLK_DIV]	= &aud_mst_d_lrclk_div.hw,
>> +	[AUD_CLKID_MST_D_LRCLK]		= &aud_mst_d_lrclk.hw,
>> +	[AUD_CLKID_TDMIN_A_SCLK_SEL]	= &aud_tdmin_a_sclk_mux.hw,
>> +	[AUD_CLKID_TDMIN_A_SCLK_PRE_EN]	= &aud_tdmin_a_sclk_pre_en.hw,
>> +	[AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &aud_tdmin_a_sclk_post_en.hw,
>> +	[AUD_CLKID_TDMIN_A_SCLK]	= &aud_tdmin_a_sclk.hw,
>> +	[AUD_CLKID_TDMIN_A_LRCLK]	= &aud_tdmin_a_lrclk.hw,
>> +	[AUD_CLKID_TDMIN_B_SCLK_SEL]	= &aud_tdmin_b_sclk_mux.hw,
>> +	[AUD_CLKID_TDMIN_B_SCLK_PRE_EN]	= &aud_tdmin_b_sclk_pre_en.hw,
>> +	[AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &aud_tdmin_b_sclk_post_en.hw,
>> +	[AUD_CLKID_TDMIN_B_SCLK]	= &aud_tdmin_b_sclk.hw,
>> +	[AUD_CLKID_TDMIN_B_LRCLK]	= &aud_tdmin_b_lrclk.hw,
>> +	[AUD_CLKID_TDMIN_LB_SCLK_SEL]	= &aud_tdmin_lb_sclk_mux.hw,
>> +	[AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &aud_tdmin_lb_sclk_pre_en.hw,
>> +	[AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &aud_tdmin_lb_sclk_post_en.hw,
>> +	[AUD_CLKID_TDMIN_LB_SCLK]	= &aud_tdmin_lb_sclk.hw,
>> +	[AUD_CLKID_TDMIN_LB_LRCLK]	= &aud_tdmin_lb_lrclk.hw,
>> +	[AUD_CLKID_TDMOUT_A_SCLK_SEL]	= &aud_tdmout_a_sclk_mux.hw,
>> +	[AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &aud_tdmout_a_sclk_pre_en.hw,
>> +	[AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &aud_tdmout_a_sclk_post_en.hw,
>> +	[AUD_CLKID_TDMOUT_A_SCLK]	= &aud_tdmout_a_sclk.hw,
>> +	[AUD_CLKID_TDMOUT_A_LRCLK]	= &aud_tdmout_a_lrclk.hw,
>> +	[AUD_CLKID_TDMOUT_B_SCLK_SEL]	= &aud_tdmout_b_sclk_mux.hw,
>> +	[AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &aud_tdmout_b_sclk_pre_en.hw,
>> +	[AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &aud_tdmout_b_sclk_post_en.hw,
>> +	[AUD_CLKID_TDMOUT_B_SCLK]	= &aud_tdmout_b_sclk.hw,
>> +	[AUD_CLKID_TDMOUT_B_LRCLK]	= &aud_tdmout_b_lrclk.hw,
>> +};
>> +
>> +static struct clk_hw *a1_audio2_hw_clks[] = {
>> +	[AUD2_CLKID_DDR_ARB]		= &aud2_ddr_arb.hw,
>> +	[AUD2_CLKID_PDM]		= &aud2_pdm.hw,
>> +	[AUD2_CLKID_TDMIN_VAD]		= &aud2_tdmin_vad.hw,
>> +	[AUD2_CLKID_TODDR_VAD]		= &aud2_toddr_vad.hw,
>> +	[AUD2_CLKID_VAD]		= &aud2_vad.hw,
>> +	[AUD2_CLKID_AUDIOTOP]		= &aud2_audiotop.hw,
>> +	[AUD2_CLKID_VAD_MCLK_SEL]	= &aud2_vad_mclk_mux.hw,
>> +	[AUD2_CLKID_VAD_MCLK_DIV]	= &aud2_vad_mclk_div.hw,
>> +	[AUD2_CLKID_VAD_MCLK]		= &aud2_vad_mclk.hw,
>> +	[AUD2_CLKID_VAD_CLK_SEL]	= &aud2_vad_clk_mux.hw,
>> +	[AUD2_CLKID_VAD_CLK_DIV]	= &aud2_vad_clk_div.hw,
>> +	[AUD2_CLKID_VAD_CLK]		= &aud2_vad_clk.hw,
>> +	[AUD2_CLKID_PDM_DCLK_SEL]	= &aud2_pdm_dclk_mux.hw,
>> +	[AUD2_CLKID_PDM_DCLK_DIV]	= &aud2_pdm_dclk_div.hw,
>> +	[AUD2_CLKID_PDM_DCLK]		= &aud2_pdm_dclk.hw,
>> +	[AUD2_CLKID_PDM_SYSCLK_SEL]	= &aud2_pdm_sysclk_mux.hw,
>> +	[AUD2_CLKID_PDM_SYSCLK_DIV]	= &aud2_pdm_sysclk_div.hw,
>> +	[AUD2_CLKID_PDM_SYSCLK]		= &aud2_pdm_sysclk.hw,
>> +};
> 
> I think you already got that comment but audio2 is a terrible name.
>> Given that the 2nd controller is mostly about VAD (PDM is there only to
> feed it) and that it is just a name, I'd prefer something like a1_audio_vad.
>

Do you mean something like this?

static struct clk_hw *a1_audio_vad_hw_clks[] = {
	[AUD_CLKID_VAD_DDR_ARB]		= &aud_vad_ddr_arb.hw,
	[AUD_CLKID_VAD_PDM]		= &aud_vad_pdm.hw,
	[AUD_CLKID_VAD_TDMIN]		= &aud_vad_tdmin.hw,
	[AUD_CLKID_VAD_TODDR]		= &aud_vad_toddr.hw,
	[AUD_CLKID_VAD]			= &aud_vad.hw,
	...


>> +
>> +static int a1_register_clk(struct platform_device *pdev, struct regmap *map,
>> +			   struct clk_hw *hw)
>> +{
>> +	struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
>> +
>> +	if (!hw)
>> +		return 0;
>> +
>> +	clk->map = map;
>> +
>> +	return devm_clk_hw_register(&pdev->dev, hw);
>> +}
> 
> Why do you have to do that and cannot do it like the rest of the
> modules ?
> 

Originally, it was used twice. But you're right, now it can be placed
into the loop in a1_audio_clkc_probe().

>> +
>> +struct a1_audio_data {
>> +	struct meson_clk_hw_data hw_clks;
>> +	int core_clkid;
>> +	const char *core_fwname;
>> +	unsigned int reset_offset;
>> +	unsigned int reset_num;
>> +};
>> +
>> +static const struct regmap_config a1_audio_regmap_cfg = {
>> +	.reg_bits = 32,
>> +	.val_bits = 32,
>> +	.reg_stride = 4,
>> +};
>> +
>> +static int a1_audio_clkc_probe(struct platform_device *pdev)
>> +{
>> +	const struct a1_audio_data *data;
>> +	struct regmap *map;
>> +	void __iomem *base;
>> +	struct clk *clk;
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	data = of_device_get_match_data(&pdev->dev);
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	if (data->core_fwname) {
> 
> That is really over-complicated for what it does  ....
> 
>> +		clk = devm_clk_get_enabled(&pdev->dev, data->core_fwname);
> 
> It does not make a lot of sense that one of the 2 controllers as 2nd
> pclk.
> > You should consider using the AUD2_CLKID_AUDIOTOP as a parent of audio
> gates instead. You would not have to that.
> 
>> +		if (IS_ERR(clk))
>> +			return PTR_ERR(clk);
>> +	}
> 
> 
> 
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	map = devm_regmap_init_mmio(&pdev->dev, base, &a1_audio_regmap_cfg);
>> +	if (IS_ERR(map))
>> +		return PTR_ERR(map);
>> +
>> +	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
>> +	if (IS_ERR(clk))
>> +		return PTR_ERR(clk);
>> +
>> +	for (i = 0; i < data->hw_clks.num; i++) {
>> +		struct clk_hw *hw = data->hw_clks.hws[i];
>> +
>> +		ret = a1_register_clk(pdev, map, hw);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
>> +					  (void *)&data->hw_clks);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!data->reset_num)
>> +		return 0;
>> +
>> +	return meson_audio_rstc_register(&pdev->dev, map, data->reset_offset,
>> +					 data->reset_num);
>> +}>
> Again this is over-complicated.> Just make an external function for the register, use it for axg, a1,
> a1-vad, then make one file for each controller. That will keep the code
> clean and readable.
> 

It's a bit unclear here. Is this a general recommendation or is it
specific to _this_ location in the code?

> Just make an external function for the register

Dou you suggest to share something like a1_register_clk()? Currently, it
is not good idea, axg-audio uses slightly different way to register
their clocks.

> keep it simple please.
> 
>> +
>> +struct a1_audio_data a1_audio_data = {
>> +	.hw_clks = {
>> +		.hws = a1_audio_hw_clks,
>> +		.num = ARRAY_SIZE(a1_audio_hw_clks),
>> +	},
>> +	.core_fwname = "core",
>> +	.reset_offset = AUDIO_SW_RESET0,
>> +	.reset_num = 32,
>> +};
>> +
>> +struct a1_audio_data a1_audio2_data = {
>> +	.hw_clks = {
>> +		.hws = a1_audio2_hw_clks,
>> +		.num = ARRAY_SIZE(a1_audio2_hw_clks),
>> +	},
>> +};
>> +
>> +static const struct of_device_id a1_audio_clkc_match_table[] = {
>> +	{
>> +		.compatible = "amlogic,a1-audio-clkc",
>> +		.data = &a1_audio_data,
>> +	},
>> +	{
>> +		.compatible = "amlogic,a1-audio2-clkc",
>> +		.data = &a1_audio2_data,
>> +	},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
>> +
>> +static struct platform_driver a1_audio_clkc_driver = {
>> +	.probe = a1_audio_clkc_probe,
>> +	.driver = {
>> +		.name = "a1-audio-clkc",
>> +		.of_match_table = a1_audio_clkc_match_table,
>> +	},
>> +};
>> +module_platform_driver(a1_audio_clkc_driver);
>> +
>> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
>> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
>> new file mode 100644
>> index 000000000000..9ea9da21ff9b
>> --- /dev/null
>> +++ b/drivers/clk/meson/a1-audio.h
>> @@ -0,0 +1,45 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +/*
>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> + *
>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> + */
>> +
>> +#ifndef __A1_AUDIO_H
>> +#define __A1_AUDIO_H
>> +
>> +#define AUDIO_CLK_GATE_EN0	0x000
>> +#define AUDIO_MCLK_A_CTRL	0x008
>> +#define AUDIO_MCLK_B_CTRL	0x00c
>> +#define AUDIO_MCLK_C_CTRL	0x010
>> +#define AUDIO_MCLK_D_CTRL	0x014
>> +#define AUDIO_MCLK_E_CTRL	0x018
>> +#define AUDIO_MCLK_F_CTRL	0x01c
>> +#define AUDIO_SW_RESET0		0x028
>> +#define AUDIO_MST_A_SCLK_CTRL0	0x040
>> +#define AUDIO_MST_A_SCLK_CTRL1	0x044
>> +#define AUDIO_MST_B_SCLK_CTRL0	0x048
>> +#define AUDIO_MST_B_SCLK_CTRL1	0x04c
>> +#define AUDIO_MST_C_SCLK_CTRL0	0x050
>> +#define AUDIO_MST_C_SCLK_CTRL1	0x054
>> +#define AUDIO_MST_D_SCLK_CTRL0	0x058
>> +#define AUDIO_MST_D_SCLK_CTRL1	0x05c
>> +#define AUDIO_CLK_TDMIN_A_CTRL	0x080
>> +#define AUDIO_CLK_TDMIN_B_CTRL	0x084
>> +#define AUDIO_CLK_TDMIN_LB_CTRL	0x08c
>> +#define AUDIO_CLK_TDMOUT_A_CTRL	0x090
>> +#define AUDIO_CLK_TDMOUT_B_CTRL	0x094
>> +#define AUDIO_CLK_SPDIFIN_CTRL	0x09c
>> +#define AUDIO_CLK_RESAMPLE_CTRL	0x0a4
>> +#define AUDIO_CLK_LOCKER_CTRL	0x0a8
>> +#define AUDIO_CLK_EQDRC_CTRL	0x0c0
>> +
>> +#define AUDIO2_CLK_GATE_EN0	0x00c
>> +#define AUDIO2_MCLK_VAD_CTRL	0x040
>> +#define AUDIO2_CLK_VAD_CTRL	0x044
>> +#define AUDIO2_CLK_PDMIN_CTRL0	0x058
>> +#define AUDIO2_CLK_PDMIN_CTRL1	0x05c
> 
> Same remark - header is useless.
> 

Do you suggest to move these defines to source file and kill this header?

>> +
>> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>> +
>> +#endif /* __A1_AUDIO_H */
> 
> 

-- 
Best regards
Jan Dakinevich

_______________________________________________
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 06/10] perf/dwc_pcie: Avoid placing cpumask on the stack
From: Shuai Xue @ 2024-04-08  0:30 UTC (permalink / raw)
  To: Dawei Li, will, mark.rutland, yury.norov, linux
  Cc: renyu.zj, yangyicong, jonathan.cameron, andersson, konrad.dybcio,
	linux-arm-kernel, linux-kernel, linux-arm-msm
In-Reply-To: <20240403155950.2068109-7-dawei.li@shingroup.cn>



On 2024/4/3 23:59, Dawei Li wrote:
> In general it's preferable to avoid placing cpumasks on the stack, as
> for large values of NR_CPUS these can consume significant amounts of
> stack space and make stack overflows more likely.
> 
> Use cpumask_any_and_but() to avoid the need for a temporary cpumask on
> the stack.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thank you.

Shuai

_______________________________________________
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 02/10] perf/alibaba_uncore_drw: Avoid placing cpumask on the stack
From: Shuai Xue @ 2024-04-08  0:28 UTC (permalink / raw)
  To: Dawei Li, will, mark.rutland, yury.norov, linux
  Cc: renyu.zj, yangyicong, jonathan.cameron, andersson, konrad.dybcio,
	linux-arm-kernel, linux-kernel, linux-arm-msm
In-Reply-To: <20240403155950.2068109-3-dawei.li@shingroup.cn>



On 2024/4/3 23:59, Dawei Li wrote:
> In general it's preferable to avoid placing cpumasks on the stack, as
> for large values of NR_CPUS these can consume significant amounts of
> stack space and make stack overflows more likely.
> 
> Use cpumask_any_and_but() to avoid the need for a temporary cpumask on
> the stack.
> 
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> ---

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

_______________________________________________
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 v9 1/7] dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
From: Bjorn Andersson @ 2024-04-08  0:07 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Stephen Boyd, Matthias Brugger, Konrad Dybcio, Dmitry Baryshkov,
	Neil Armstrong, AngeloGioacchino Del Regno, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Srini Kandagatla, Johan Hovold,
	David Collins, linux-kernel, linux-arm-kernel, linux-arm-msm,
	linux-mediatek, devicetree, Krzysztof Kozlowski
In-Reply-To: <20240407-spmi-multi-master-support-v9-1-fa151c1391f3@linaro.org>

On Sun, Apr 07, 2024 at 07:23:21PM +0300, Abel Vesa wrote:
> Add dedicated schema for X1E80100 PMIC ARB (v7) as it allows multiple
> buses by declaring them as child nodes.
> 

But is this really a "dedicated schema for X1E80100"? Isn't it "the
schema for all multi-bus controllers"?

I.e. isn't this a "dedicated schema for all platforms starting with
SM8450"?

Can you please use the commit message to document the actual reason why
you choose to create a dedicated schema for this? Is it simply to avoid
having to schema with either pmics or multiple buses as children?

Regards,
Bjorn

_______________________________________________
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 v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Peng Fan @ 2024-04-07 23:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Peng Fan (OSS), Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Sudeep Holla,
	Cristian Marussi
  Cc: devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <5b9e0e44-0b9c-44fc-9d18-21c47b46dc63@kernel.org>

> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> additionalProperties to true
> 
> On 07/04/2024 12:04, Peng Fan wrote:
> >> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> >> additionalProperties to true
> >>
> >> On 07/04/2024 02:37, Peng Fan wrote:
> >>>> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> >>>> additionalProperties to true
> >>>>
> >>>> On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> >>>>> From: Peng Fan <peng.fan@nxp.com>
> >>>>>
> >>>>> When adding vendor extension protocols, there is dt-schema warning:
> >>>>> "
> >>>>> imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not
> >>>>> match any of the regexes: 'pinctrl-[0-9]+'
> >>>>> "
> >>>>>
> >>>>> Set additionalProperties to true to address the issue.
> >>>>
> >>>> I do not see anything addressed here, except making the binding
> >>>> accepting anything anywhere...
> >>>
> >>> I not wanna add vendor protocols in arm,scmi.yaml, so will introduce
> >>> a new yaml imx.scmi.yaml which add i.MX SCMI protocol extension.
> >>>
> >>> With additionalProperties set to false, I not know how, please suggest.
> >>
> >> First of all, you cannot affect negatively existing devices (their
> >> bindings) and your patch does exactly that. This should make you
> >> thing what is the correct approach...
> >>
> >> Rob gave you the comment about missing compatible - you still did not
> >> address that.
> >
> > I added the compatible in patch 2/6 in the examples "compatible =
> "arm,scmi";"
> 
> So you claim that your vendor extensions are the same or fully compatible
> with arm,scmi and you add nothing... Are your extensions/protocol valid for
> arm,scmi?

Yes. They are valid for arm,scmi.

 If yes, why is this in separate binding. If no, why you use someone
> else's compatible?

Per SCMI Spec
0x80-0xFF: Reserved for vendor or platform-specific extensions to
this interface

i.MX use 0x81 for BBM, 0x84 for MISC. But other vendors will use
the id for their own protocol.

I use a separate binding here is to avoid add more vendor stuff
in arm,scmi.yaml. Otherwise we will have to add a list as:
if nxp
xxx
else if qcom
xxx
else if xx
yyy.

I could add back i.mx extension to arm,scmi.yaml if people
agree.

Thanks
Peng.

> 
> Maybe your binding is correct, feel free to convince me (and read first writing
> bindings).
> 
> 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 v3 1/2] dt-bindings: mailbox: arm,mhuv3: Add bindings
From: Jassi Brar @ 2024-04-07 23:38 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla, robh+dt,
	krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <20240404062347.3219795-2-cristian.marussi@arm.com>

On Thu, Apr 4, 2024 at 1:25 AM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
> Add bindings for the ARM MHUv3 Mailbox controller.
>
> 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 ?


> +  '#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.

Cheers!
Jassi

_______________________________________________
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 0/1] phy: freescale: imx8m-pcie: facing pcie link-up instability
From: Marcel Ziswiler @ 2024-04-07 22:22 UTC (permalink / raw)
  To: linux-phy@lists.infradead.org, vkoul@kernel.org
  Cc: hongxing.zhu@nxp.com, linux-imx@nxp.com, robh@kernel.org,
	kishon@kernel.org, heiko@sntech.de, l.stach@pengutronix.de,
	yang.lee@linux.alibaba.com, linux-kernel@vger.kernel.org,
	tharvey@gateworks.com, shawnguo@kernel.org, kernel@pengutronix.de,
	s.hauer@pengutronix.de, mkl@pengutronix.de,
	linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev,
	festevam@gmail.com
In-Reply-To: <171239511427.352254.8525089049107113141.b4-ty@kernel.org>

Hi Vinod

On Sat, 2024-04-06 at 14:48 +0530, Vinod Koul wrote:
> 
> On Fri, 22 Mar 2024 14:06:31 +0100, Marcel Ziswiler wrote:
> > In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> > While this setup has proven very stable overall we noticed upstream on
> > the i.MX8M Mini fails quite regularly (about 50/50) to bring up the PCIe
> > link while with NXP's downstream BSP 5.15.71_2.2.2 it always works. As
> > that old downstream stuff was quite different, I first also tried NXP's
> > latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view is
> > fairly vanilla, however, also there the PCIe link-up was not stable.
> > Comparing and debugging I noticed that upstream explicitly configures
> > the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> > downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> > Unfortunately, the TRM does not mention any further details about this
> > register (both for the i.MX 8M Mini as well as the Plus). Maybe somebody
> > from NXP could further comment on this?
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
>       commit: 3a161017f1de55cc48be81f6156004c151f32677

Sorry, but it is slightly confusing whether v1 or v2 now got applied. I believe v1 but then only the commit
messages differ. However, please note that only v2 included information on how to proceed concerning
backporting to stable 6.1.x.

Thanks!

> Best regards,
> --
> -Vinod

Cheers

Marcel
_______________________________________________
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 v2] phy: freescale: imx8m-pcie: fix pcie link-up instability
From: Marcel Ziswiler @ 2024-04-07 22:23 UTC (permalink / raw)
  To: linux-phy@lists.infradead.org, vkoul@kernel.org
  Cc: hongxing.zhu@nxp.com, linux-imx@nxp.com, robh@kernel.org,
	kishon@kernel.org, heiko@sntech.de, l.stach@pengutronix.de,
	stable@vger.kernel.org, yang.lee@linux.alibaba.com,
	sergio.paracuellos@gmail.com, linux-kernel@vger.kernel.org,
	tharvey@gateworks.com, shawnguo@kernel.org, kernel@pengutronix.de,
	s.hauer@pengutronix.de, linux-arm-kernel@lists.infradead.org,
	imx@lists.linux.dev, festevam@gmail.com
In-Reply-To: <171239512402.352254.17030670986383011631.b4-ty@kernel.org>

Hi Vinod

On Sat, 2024-04-06 at 14:48 +0530, Vinod Koul wrote:
> 
> On Wed, 27 Mar 2024 08:19:37 +0100, Marcel Ziswiler wrote:
> > On the i.MX 8M Mini, the AUX_PLL_REFCLK_SEL has to be left at its reset
> > default of AUX_IN (PLL clock).
> > 
> > Background Information:
> > In our automated testing setup, we use Delock Mini-PCIe SATA cards [1].
> > While this setup has proven very stable overall we noticed upstream on
> > the i.MX 8M Mini fails quite regularly (about 50/50) to bring up the
> > PCIe link while with NXP's downstream BSP 5.15.71_2.2.2 it always works.
> > As that old downstream stuff was quite different, I first also tried
> > NXP's latest downstream BSP 6.1.55_2.2.0 which from a PCIe point of view
> > is fairly vanilla, however, also there the PCIe link-up was not stable.
> > Comparing and debugging I noticed that upstream explicitly configures
> > the AUX_PLL_REFCLK_SEL to I_PLL_REFCLK_FROM_SYSPLL while working
> > downstream [2] leaving it at reset defaults of AUX_IN (PLL clock).
> > Unfortunately, the TRM does not mention any further details about this
> > register (both for the i.MX 8M Mini as well as the Plus).
> > NXP confirmed their validation codes for the i.MX8MM PCIe doesn't
> > configure cmn_reg063 (offset: 0x18C).
> > BTW: On the i.MX 8M Plus we have not seen any issues with PCIe with the
> > exact same setup which is why I left it unchanged.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] phy: freescale: imx8m-pcie: fix pcie link-up instability
>       commit: 3a161017f1de55cc48be81f6156004c151f32677

Sorry, but it is slightly confusing whether v1 or v2 now got applied. I believe v1 but then only the commit
messages differ. However, please note that only v2 included information on how to proceed concerning
backporting to stable 6.1.x.

Thanks!

> Best regards,
> --
> -Vinod

Cheers

Marcel
_______________________________________________
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 net] net: dsa: mt7530: trap link-local frames regardless of ST Port State
From: Arınç ÜNAL @ 2024-04-07 20:16 UTC (permalink / raw)
  To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20240407-b4-for-net-mt7530-fix-link-local-when-stp-discarding-v1-1-b4b20ac93457@arinc9.com>

On 7.04.2024 21:11, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> In Clause 5 of IEEE Std 802-2014, two sublayers of the data link layer
> (DLL) of the Open Systems Interconnection basic reference model (OSI/RM)
> are described; the medium access control (MAC) and logical link control
> (LLC) sublayers. The MAC sublayer is the one facing the physical layer.

I forgot to add a Fixes: tag. I'll send v2 in a few days.

Arınç

_______________________________________________
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/2] thermal/drivers/mediatek/lvts_thermal: Make debugfs related fields more consistent
From: Christophe JAILLET @ 2024-04-07 20:01 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-pm,
	linux-arm-kernel, linux-mediatek

The debugfs code is only generated if CONFIG_MTK_LVTS_THERMAL_DEBUGFS is
defined.

So 'dom_dentry' should be included in the 'struct lvts_domain' with the
same condition, instead of CONFIG_DEBUG_FS.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/thermal/mediatek/lvts_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index fd4bd650c77a..3003dc350766 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -150,7 +150,7 @@ struct lvts_domain {
 	void __iomem *base;
 	size_t calib_len;
 	u8 *calib;
-#ifdef CONFIG_DEBUG_FS
+#ifdef CONFIG_MTK_LVTS_THERMAL_DEBUGFS
 	struct dentry *dom_dentry;
 #endif
 };
-- 
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 related

* [PATCH 2/2] thermal/drivers/mediatek/lvts_thermal: Improve some memory allocation
From: Christophe JAILLET @ 2024-04-07 20:01 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-pm,
	linux-arm-kernel, linux-mediatek
In-Reply-To: <42a87fb9837f1760d1ad4eb7162a7536785dc6f5.1712520052.git.christophe.jaillet@wanadoo.fr>

lvts_debugfs_init() is called with lvts_td->num_lvts_ctrl being 2, 3 or 4.
sizeof(struct debugfs_regset32) is small. 32 byres on a x86_64.

So, given the overhead of devm_kzalloc(), it is better to allocate all
needed regset at once.

The overhead of devm_kzalloc() is 40 bytes on a x86_64, and because of
rounding in memory allocation, it leads to:
 2 * (32 + 40) = 2 * 72	--> 2 96 bytes allocations for a total of 192 bytes
 3 * (32 + 40) = 3 * 72	--> 3 96 bytes allocations for a total of 288 bytes
 4 * (32 + 40) = 4 * 72	--> 4 96 bytes allocations for a total of 384 bytes

using a single devm_kcalloc():
 2 * 32 + 40 = 104	--> 1 allocation for a total of 128
 3 * 32 + 40 = 136	--> 1 allocation for a total of 192
 4 * 32 + 40 = 168	--> 1 allocation for a total of 192

So, this saves both a few bytes and reduce memory fragmentation.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/thermal/mediatek/lvts_thermal.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 3003dc350766..b133f731c5ba 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -204,7 +204,7 @@ static const struct debugfs_reg32 lvts_regs[] = {
 
 static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 {
-	struct debugfs_regset32 *regset;
+	struct debugfs_regset32 *regsets;
 	struct lvts_ctrl *lvts_ctrl;
 	struct dentry *dentry;
 	char name[64];
@@ -214,8 +214,14 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 	if (IS_ERR(lvts_td->dom_dentry))
 		return 0;
 
+	regsets = devm_kcalloc(dev, lvts_td->num_lvts_ctrl,
+			       sizeof(*regsets), GFP_KERNEL);
+	if (!regsets)
+		return 0;
+
 	for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
 
+		struct debugfs_regset32 *regset = &regsets[i];
 		lvts_ctrl = &lvts_td->lvts_ctrl[i];
 
 		sprintf(name, "controller%d", i);
@@ -223,10 +229,6 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 		if (IS_ERR(dentry))
 			continue;
 
-		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
-		if (!regset)
-			continue;
-
 		regset->base = lvts_ctrl->base;
 		regset->regs = lvts_regs;
 		regset->nregs = ARRAY_SIZE(lvts_regs);
-- 
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 related

* [PATCH net] net: dsa: mt7530: trap link-local frames regardless of ST Port State
From: Arınç ÜNAL via B4 Relay @ 2024-04-07 18:11 UTC (permalink / raw)
  To: Daniel Golle, DENG Qingfang, Sean Wang, Andrew Lunn,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Bartel Eerdekens, mithat.guner, erkin.bozoglu, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Arınç ÜNAL

From: Arınç ÜNAL <arinc.unal@arinc9.com>

In Clause 5 of IEEE Std 802-2014, two sublayers of the data link layer
(DLL) of the Open Systems Interconnection basic reference model (OSI/RM)
are described; the medium access control (MAC) and logical link control
(LLC) sublayers. The MAC sublayer is the one facing the physical layer.

In 8.2 of IEEE Std 802.1Q-2022, the Bridge architecture is described. A
Bridge component comprises a MAC Relay Entity for interconnecting the Ports
of the Bridge, at least two Ports, and higher layer entities with at least
a Spanning Tree Protocol Entity included.

Each Bridge Port also functions as an end station and shall provide the MAC
Service to an LLC Entity. Each instance of the MAC Service is provided to a
distinct LLC Entity that supports protocol identification, multiplexing,
and demultiplexing, for protocol data unit (PDU) transmission and reception
by one or more higher layer entities.

It is described in 8.13.9 of IEEE Std 802.1Q-2022 that in a Bridge, the LLC
Entity associated with each Bridge Port is modeled as being directly
connected to the attached Local Area Network (LAN).

On the switch with CPU port architecture, CPU port functions as Management
Port, and the Management Port functionality is provided by software which
functions as an end station. Software is connected to an IEEE 802 LAN that
is wholly contained within the system that incorporates the Bridge.
Software provides access to the LLC Entity associated with each Bridge Port
by the value of the source port field on the special tag on the frame
received by software.

We call frames that carry control information to determine the active
topology and current extent of each Virtual Local Area Network (VLAN),
i.e., spanning tree or Shortest Path Bridging (SPB) and Multiple VLAN
Registration Protocol Data Units (MVRPDUs), and frames from other link
constrained protocols, such as Extensible Authentication Protocol over LAN
(EAPOL) and Link Layer Discovery Protocol (LLDP), link-local frames. They
are not forwarded by a Bridge. Permanently configured entries in the
filtering database (FDB) ensure that such frames are discarded by the
Forwarding Process. In 8.6.3 of IEEE Std 802.1Q-2022, this is described in
detail:

Each of the reserved MAC addresses specified in Table 8-1
(01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]) shall be
permanently configured in the FDB in C-VLAN components and ERs.

Each of the reserved MAC addresses specified in Table 8-2
(01-80-C2-00-00-[01,02,03,04,05,06,07,08,09,0A,0E]) shall be permanently
configured in the FDB in S-VLAN components.

Each of the reserved MAC addresses specified in Table 8-3
(01-80-C2-00-00-[01,02,04,0E]) shall be permanently configured in the FDB
in TPMR components.

The FDB entries for reserved MAC addresses shall specify filtering for all
Bridge Ports and all VIDs. Management shall not provide the capability to
modify or remove entries for reserved MAC addresses.

The addresses in Table 8-1, Table 8-2, and Table 8-3 determine the scope of
propagation of PDUs within a Bridged Network, as follows:

  The Nearest Bridge group address (01-80-C2-00-00-0E) is an address that
  no conformant Two-Port MAC Relay (TPMR) component, Service VLAN (S-VLAN)
  component, Customer VLAN (C-VLAN) component, or MAC Bridge can forward.
  PDUs transmitted using this destination address, or any other addresses
  that appear in Table 8-1, Table 8-2, and Table 8-3
  (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]), can
  therefore travel no further than those stations that can be reached via a
  single individual LAN from the originating station.

  The Nearest non-TPMR Bridge group address (01-80-C2-00-00-03), is an
  address that no conformant S-VLAN component, C-VLAN component, or MAC
  Bridge can forward; however, this address is relayed by a TPMR component.
  PDUs using this destination address, or any of the other addresses that
  appear in both Table 8-1 and Table 8-2 but not in Table 8-3
  (01-80-C2-00-00-[00,03,05,06,07,08,09,0A,0B,0C,0D,0F]), will be relayed
  by any TPMRs but will propagate no further than the nearest S-VLAN
  component, C-VLAN component, or MAC Bridge.

  The Nearest Customer Bridge group address (01-80-C2-00-00-00) is an
  address that no conformant C-VLAN component, MAC Bridge can forward;
  however, it is relayed by TPMR components and S-VLAN components. PDUs
  using this destination address, or any of the other addresses that appear
  in Table 8-1 but not in either Table 8-2 or Table 8-3
  (01-80-C2-00-00-[00,0B,0C,0D,0F]), will be relayed by TPMR components and
  S-VLAN components but will propagate no further than the nearest C-VLAN
  component or MAC Bridge.

Because the LLC Entity associated with each Bridge Port is provided via CPU
port, we must not filter these frames but forward them to CPU port.

In a Bridge, the transmission Port is majorly decided by ingress and egress
rules, FDB, and spanning tree Port State functions of the Forwarding
Process. For link-local frames, only CPU port should be designated as
destination port in the FDB, and the other functions of the Forwarding
Process must not interfere with the decision of the transmission Port. We
call this process trapping frames to CPU port.

Therefore, on the switch with CPU port architecture, link-local frames must
be trapped to CPU port, and certain link-local frames received by a Port of
a Bridge comprising a TPMR component or an S-VLAN component must be
excluded from it.

A Bridge of the switch with CPU architecture cannot comprise a non-Two-Port
MAC Relay (TPMR) component as TPMR component is only supposed to support a
subset of the functionality of a MAC Bridge. A Bridge comprising two ports
(Management Port doesn't count) of this architecture will either function
as a standard MAC Bridge or a standard VLAN Bridge.

Therefore, a Bridge of this architecture can only comprise S-VLAN
components, C-VLAN components, or MAC Bridge components. Since there's no
TPMR component, we don't need to relay PDUs using the destination addresses
specified on the Nearest non-TPMR section, and the proportion of the
Nearest Customer Bridge section where they must be relayed by TPMR
components.

One option to trap link-local frames to CPU port is to add static FDB
entries with CPU port designated as destination port. However, because that
Independent VLAN Learning (IVL) is being used on every VID, each entry only
applies to a single VLAN Identifier (VID). For a Bridge comprising a MAC
Bridge component or a C-VLAN component, there would have to be 16 times
4096 entries. This switch intellectual property can only hold a maximum of
2048 entries. Using this option, there also isn't a mechanism to prevent
link-local frames from being discarded when the spanning tree Port State of
the reception Port is discarding.

The remaining option is to utilise the BPC, RGAC1, RGAC2, RGAC3, and RGAC4
registers. Whilst this applies to every VID, it doesn't contain all of the
reserved MAC addresses without affecting the remaining Standard Group MAC
Addresses. The REV_UN frame tag utilised using the RGAC4 register covers
the remaining 01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F] destination
addresses. It also includes the 01-80-C2-00-00-22 to 01-80-C2-00-00-FF
destination addresses which may be relayed by MAC Bridges or VLAN Bridges.
The latter option provides better but not complete conformance.

This switch intellectual property also does not provide a mechanism to trap
link-local frames with specific destination addresses to CPU port by
Bridge, to conform to the filtering rules for the distinct Bridge
components.

Therefore, regardless of the type of the Bridge component, link-local
frames with these destination addresses will be trapped to CPU port:

01-80-C2-00-00-[00,01,02,03,0E]

In a Bridge comprising a MAC Bridge component or a C-VLAN component:

  Link-local frames with these destination addresses won't be trapped to
  CPU port which won't conform to IEEE Std 802.1Q-2022:

  01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F]

In a Bridge comprising an S-VLAN component:

  Link-local frames with these destination addresses will be trapped to CPU
  port which won't conform to IEEE Std 802.1Q-2022:

  01-80-C2-00-00-00

  Link-local frames with these destination addresses won't be trapped to
  CPU port which won't conform to IEEE Std 802.1Q-2022:

  01-80-C2-00-00-[04,05,06,07,08,09,0A]

Currently on this switch intellectual property, if the spanning tree Port
State of the reception Port is discarding, link-local frames will be
discarded.

To trap link-local frames regardless of the spanning tree Port State, make
the switch regard them as BPDU. This switch intellectual property only lets
the frames regarded as BPDU bypass the spanning tree Port State function of
the Forwarding Process.

With this change, the only remaining interference left is the ingress
rules. When the reception Port has no PVID assigned on software,
VLAN-untagged frames won't be allowed in. There doesn't seem to be a
mechanism on the switch intellectual property to have link-local frames
bypass this function of the Forwarding Process.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 229 ++++++++++++++++++++++++++++++++++++++++-------
 drivers/net/dsa/mt7530.h |   5 ++
 2 files changed, 200 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 1035820c2377..254501f63731 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -950,20 +950,173 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 	mutex_unlock(&priv->reg_mutex);
 }
 
-/* On page 205, section "8.6.3 Frame filtering" of the active standard, IEEE Std
- * 802.1Q™-2022, it is stated that frames with 01:80:C2:00:00:00-0F as MAC DA
- * must only be propagated to C-VLAN and MAC Bridge components. That means
- * VLAN-aware and VLAN-unaware bridges. On the switch designs with CPU ports,
- * these frames are supposed to be processed by the CPU (software). So we make
- * the switch only forward them to the CPU port. And if received from a CPU
- * port, forward to a single port. The software is responsible of making the
- * switch conform to the latter by setting a single port as destination port on
- * the special tag.
+/* In Clause 5 of IEEE Std 802-2014, two sublayers of the data link layer (DLL)
+ * of the Open Systems Interconnection basic reference model (OSI/RM) are
+ * described; the medium access control (MAC) and logical link control (LLC)
+ * sublayers. The MAC sublayer is the one facing the physical layer.
  *
- * This switch intellectual property cannot conform to this part of the standard
- * fully. Whilst the REV_UN frame tag covers the remaining :04-0D and :0F MAC
- * DAs, it also includes :22-FF which the scope of propagation is not supposed
- * to be restricted for these MAC DAs.
+ * In 8.2 of IEEE Std 802.1Q-2022, the Bridge architecture is described. A
+ * Bridge component comprises a MAC Relay Entity for interconnecting the Ports
+ * of the Bridge, at least two Ports, and higher layer entities with at least a
+ * Spanning Tree Protocol Entity included.
+ *
+ * Each Bridge Port also functions as an end station and shall provide the MAC
+ * Service to an LLC Entity. Each instance of the MAC Service is provided to a
+ * distinct LLC Entity that supports protocol identification, multiplexing, and
+ * demultiplexing, for protocol data unit (PDU) transmission and reception by
+ * one or more higher layer entities.
+ *
+ * It is described in 8.13.9 of IEEE Std 802.1Q-2022 that in a Bridge, the LLC
+ * Entity associated with each Bridge Port is modeled as being directly
+ * connected to the attached Local Area Network (LAN).
+ *
+ * On the switch with CPU port architecture, CPU port functions as Management
+ * Port, and the Management Port functionality is provided by software which
+ * functions as an end station. Software is connected to an IEEE 802 LAN that is
+ * wholly contained within the system that incorporates the Bridge. Software
+ * provides access to the LLC Entity associated with each Bridge Port by the
+ * value of the source port field on the special tag on the frame received by
+ * software.
+ *
+ * We call frames that carry control information to determine the active
+ * topology and current extent of each Virtual Local Area Network (VLAN), i.e.,
+ * spanning tree or Shortest Path Bridging (SPB) and Multiple VLAN Registration
+ * Protocol Data Units (MVRPDUs), and frames from other link constrained
+ * protocols, such as Extensible Authentication Protocol over LAN (EAPOL) and
+ * Link Layer Discovery Protocol (LLDP), link-local frames. They are not
+ * forwarded by a Bridge. Permanently configured entries in the filtering
+ * database (FDB) ensure that such frames are discarded by the Forwarding
+ * Process. In 8.6.3 of IEEE Std 802.1Q-2022, this is described in detail:
+ *
+ * Each of the reserved MAC addresses specified in Table 8-1
+ * (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]) shall be
+ * permanently configured in the FDB in C-VLAN components and ERs.
+ *
+ * Each of the reserved MAC addresses specified in Table 8-2
+ * (01-80-C2-00-00-[01,02,03,04,05,06,07,08,09,0A,0E]) shall be permanently
+ * configured in the FDB in S-VLAN components.
+ *
+ * Each of the reserved MAC addresses specified in Table 8-3
+ * (01-80-C2-00-00-[01,02,04,0E]) shall be permanently configured in the FDB in
+ * TPMR components.
+ *
+ * The FDB entries for reserved MAC addresses shall specify filtering for all
+ * Bridge Ports and all VIDs. Management shall not provide the capability to
+ * modify or remove entries for reserved MAC addresses.
+ *
+ * The addresses in Table 8-1, Table 8-2, and Table 8-3 determine the scope of
+ * propagation of PDUs within a Bridged Network, as follows:
+ *
+ *   The Nearest Bridge group address (01-80-C2-00-00-0E) is an address that no
+ *   conformant Two-Port MAC Relay (TPMR) component, Service VLAN (S-VLAN)
+ *   component, Customer VLAN (C-VLAN) component, or MAC Bridge can forward.
+ *   PDUs transmitted using this destination address, or any other addresses
+ *   that appear in Table 8-1, Table 8-2, and Table 8-3
+ *   (01-80-C2-00-00-[00,01,02,03,04,05,06,07,08,09,0A,0B,0C,0D,0E,0F]), can
+ *   therefore travel no further than those stations that can be reached via a
+ *   single individual LAN from the originating station.
+ *
+ *   The Nearest non-TPMR Bridge group address (01-80-C2-00-00-03), is an
+ *   address that no conformant S-VLAN component, C-VLAN component, or MAC
+ *   Bridge can forward; however, this address is relayed by a TPMR component.
+ *   PDUs using this destination address, or any of the other addresses that
+ *   appear in both Table 8-1 and Table 8-2 but not in Table 8-3
+ *   (01-80-C2-00-00-[00,03,05,06,07,08,09,0A,0B,0C,0D,0F]), will be relayed by
+ *   any TPMRs but will propagate no further than the nearest S-VLAN component,
+ *   C-VLAN component, or MAC Bridge.
+ *
+ *   The Nearest Customer Bridge group address (01-80-C2-00-00-00) is an address
+ *   that no conformant C-VLAN component, MAC Bridge can forward; however, it is
+ *   relayed by TPMR components and S-VLAN components. PDUs using this
+ *   destination address, or any of the other addresses that appear in Table 8-1
+ *   but not in either Table 8-2 or Table 8-3 (01-80-C2-00-00-[00,0B,0C,0D,0F]),
+ *   will be relayed by TPMR components and S-VLAN components but will propagate
+ *   no further than the nearest C-VLAN component or MAC Bridge.
+ *
+ * Because the LLC Entity associated with each Bridge Port is provided via CPU
+ * port, we must not filter these frames but forward them to CPU port.
+ *
+ * In a Bridge, the transmission Port is majorly decided by ingress and egress
+ * rules, FDB, and spanning tree Port State functions of the Forwarding Process.
+ * For link-local frames, only CPU port should be designated as destination port
+ * in the FDB, and the other functions of the Forwarding Process must not
+ * interfere with the decision of the transmission Port. We call this process
+ * trapping frames to CPU port.
+ *
+ * Therefore, on the switch with CPU port architecture, link-local frames must
+ * be trapped to CPU port, and certain link-local frames received by a Port of a
+ * Bridge comprising a TPMR component or an S-VLAN component must be excluded
+ * from it.
+ *
+ * A Bridge of the switch with CPU architecture cannot comprise a non-Two-Port
+ * MAC Relay (TPMR) component as TPMR component is only supposed to support a
+ * subset of the functionality of a MAC Bridge. A Bridge comprising two ports
+ * (Management Port doesn't count) of this architecture will either function as
+ * a standard MAC Bridge or a standard VLAN Bridge.
+ *
+ * Therefore, a Bridge of this architecture can only comprise S-VLAN components,
+ * C-VLAN components, or MAC Bridge components. Since there's no TPMR component,
+ * we don't need to relay PDUs using the destination addresses specified on the
+ * Nearest non-TPMR section, and the proportion of the Nearest Customer Bridge
+ * section where they must be relayed by TPMR components.
+ *
+ * One option to trap link-local frames to CPU port is to add static FDB entries
+ * with CPU port designated as destination port. However, because that
+ * Independent VLAN Learning (IVL) is being used on every VID, each entry only
+ * applies to a single VLAN Identifier (VID). For a Bridge comprising a MAC
+ * Bridge component or a C-VLAN component, there would have to be 16 times 4096
+ * entries. This switch intellectual property can only hold a maximum of 2048
+ * entries. Using this option, there also isn't a mechanism to prevent
+ * link-local frames from being discarded when the spanning tree Port State of
+ * the reception Port is discarding.
+ *
+ * The remaining option is to utilise the BPC, RGAC1, RGAC2, RGAC3, and RGAC4
+ * registers. Whilst this applies to every VID, it doesn't contain all of the
+ * reserved MAC addresses without affecting the remaining Standard Group MAC
+ * Addresses. The REV_UN frame tag utilised using the RGAC4 register covers the
+ * remaining 01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F] destination
+ * addresses. It also includes the 01-80-C2-00-00-22 to 01-80-C2-00-00-FF
+ * destination addresses which may be relayed by MAC Bridges or VLAN Bridges.
+ * The latter option provides better but not complete conformance.
+ *
+ * This switch intellectual property also does not provide a mechanism to trap
+ * link-local frames with specific destination addresses to CPU port by Bridge,
+ * to conform to the filtering rules for the distinct Bridge components.
+ *
+ * Therefore, regardless of the type of the Bridge component, link-local frames
+ * with these destination addresses will be trapped to CPU port:
+ *
+ * 01-80-C2-00-00-[00,01,02,03,0E]
+ *
+ * In a Bridge comprising a MAC Bridge component or a C-VLAN component:
+ *
+ *   Link-local frames with these destination addresses won't be trapped to CPU
+ *   port which won't conform to IEEE Std 802.1Q-2022:
+ *
+ *   01-80-C2-00-00-[04,05,06,07,08,09,0A,0B,0C,0D,0F]
+ *
+ * In a Bridge comprising an S-VLAN component:
+ *
+ *   Link-local frames with these destination addresses will be trapped to CPU
+ *   port which won't conform to IEEE Std 802.1Q-2022:
+ *
+ *   01-80-C2-00-00-00
+ *
+ *   Link-local frames with these destination addresses won't be trapped to CPU
+ *   port which won't conform to IEEE Std 802.1Q-2022:
+ *
+ *   01-80-C2-00-00-[04,05,06,07,08,09,0A]
+ *
+ * To trap link-local frames to CPU port as conformant as this switch
+ * intellectual property can allow, link-local frames are made to be regarded as
+ * BPDU. This is because this switch intellectual property only lets the frames
+ * regarded as BPDU bypass the spanning tree Port State function of the
+ * Forwarding Process.
+ *
+ * The only remaining interference left is the ingress rules. When the reception
+ * Port has no PVID assigned on software, VLAN-untagged frames won't be allowed
+ * in. There doesn't seem to be a mechanism on the switch intellectual property
+ * to have link-local frames bypass this function of the Forwarding Process.
  */
 static void
 mt753x_trap_frames(struct mt7530_priv *priv)
@@ -971,35 +1124,43 @@ mt753x_trap_frames(struct mt7530_priv *priv)
 	/* Trap 802.1X PAE frames and BPDUs to the CPU port(s) and egress them
 	 * VLAN-untagged.
 	 */
-	mt7530_rmw(priv, MT753X_BPC, MT753X_PAE_EG_TAG_MASK |
-		   MT753X_PAE_PORT_FW_MASK | MT753X_BPDU_EG_TAG_MASK |
-		   MT753X_BPDU_PORT_FW_MASK,
-		   MT753X_PAE_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_PAE_PORT_FW(MT753X_BPDU_CPU_ONLY) |
-		   MT753X_BPDU_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_BPDU_CPU_ONLY);
+	mt7530_rmw(priv, MT753X_BPC,
+		   MT753X_PAE_BPDU_FR | MT753X_PAE_EG_TAG_MASK |
+			   MT753X_PAE_PORT_FW_MASK | MT753X_BPDU_EG_TAG_MASK |
+			   MT753X_BPDU_PORT_FW_MASK,
+		   MT753X_PAE_BPDU_FR |
+			   MT753X_PAE_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_PAE_PORT_FW(MT753X_BPDU_CPU_ONLY) |
+			   MT753X_BPDU_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_BPDU_CPU_ONLY);
 
 	/* Trap frames with :01 and :02 MAC DAs to the CPU port(s) and egress
 	 * them VLAN-untagged.
 	 */
-	mt7530_rmw(priv, MT753X_RGAC1, MT753X_R02_EG_TAG_MASK |
-		   MT753X_R02_PORT_FW_MASK | MT753X_R01_EG_TAG_MASK |
-		   MT753X_R01_PORT_FW_MASK,
-		   MT753X_R02_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_R02_PORT_FW(MT753X_BPDU_CPU_ONLY) |
-		   MT753X_R01_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_BPDU_CPU_ONLY);
+	mt7530_rmw(priv, MT753X_RGAC1,
+		   MT753X_R02_BPDU_FR | MT753X_R02_EG_TAG_MASK |
+			   MT753X_R02_PORT_FW_MASK | MT753X_R01_BPDU_FR |
+			   MT753X_R01_EG_TAG_MASK | MT753X_R01_PORT_FW_MASK,
+		   MT753X_R02_BPDU_FR |
+			   MT753X_R02_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_R02_PORT_FW(MT753X_BPDU_CPU_ONLY) |
+			   MT753X_R01_BPDU_FR |
+			   MT753X_R01_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_BPDU_CPU_ONLY);
 
 	/* Trap frames with :03 and :0E MAC DAs to the CPU port(s) and egress
 	 * them VLAN-untagged.
 	 */
-	mt7530_rmw(priv, MT753X_RGAC2, MT753X_R0E_EG_TAG_MASK |
-		   MT753X_R0E_PORT_FW_MASK | MT753X_R03_EG_TAG_MASK |
-		   MT753X_R03_PORT_FW_MASK,
-		   MT753X_R0E_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_R0E_PORT_FW(MT753X_BPDU_CPU_ONLY) |
-		   MT753X_R03_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
-		   MT753X_BPDU_CPU_ONLY);
+	mt7530_rmw(priv, MT753X_RGAC2,
+		   MT753X_R0E_BPDU_FR | MT753X_R0E_EG_TAG_MASK |
+			   MT753X_R0E_PORT_FW_MASK | MT753X_R03_BPDU_FR |
+			   MT753X_R03_EG_TAG_MASK | MT753X_R03_PORT_FW_MASK,
+		   MT753X_R0E_BPDU_FR |
+			   MT753X_R0E_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_R0E_PORT_FW(MT753X_BPDU_CPU_ONLY) |
+			   MT753X_R03_BPDU_FR |
+			   MT753X_R03_EG_TAG(MT7530_VLAN_EG_UNTAGGED) |
+			   MT753X_BPDU_CPU_ONLY);
 }
 
 static void
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index d17b318e6ee4..2deffe741484 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -65,6 +65,7 @@ enum mt753x_id {
 
 /* Registers for BPDU and PAE frame control*/
 #define MT753X_BPC			0x24
+#define  MT753X_PAE_BPDU_FR		BIT(25)
 #define  MT753X_PAE_EG_TAG_MASK		GENMASK(24, 22)
 #define  MT753X_PAE_EG_TAG(x)		FIELD_PREP(MT753X_PAE_EG_TAG_MASK, x)
 #define  MT753X_PAE_PORT_FW_MASK	GENMASK(18, 16)
@@ -75,20 +76,24 @@ enum mt753x_id {
 
 /* Register for :01 and :02 MAC DA frame control */
 #define MT753X_RGAC1			0x28
+#define  MT753X_R02_BPDU_FR		BIT(25)
 #define  MT753X_R02_EG_TAG_MASK		GENMASK(24, 22)
 #define  MT753X_R02_EG_TAG(x)		FIELD_PREP(MT753X_R02_EG_TAG_MASK, x)
 #define  MT753X_R02_PORT_FW_MASK	GENMASK(18, 16)
 #define  MT753X_R02_PORT_FW(x)		FIELD_PREP(MT753X_R02_PORT_FW_MASK, x)
+#define  MT753X_R01_BPDU_FR		BIT(9)
 #define  MT753X_R01_EG_TAG_MASK		GENMASK(8, 6)
 #define  MT753X_R01_EG_TAG(x)		FIELD_PREP(MT753X_R01_EG_TAG_MASK, x)
 #define  MT753X_R01_PORT_FW_MASK	GENMASK(2, 0)
 
 /* Register for :03 and :0E MAC DA frame control */
 #define MT753X_RGAC2			0x2c
+#define  MT753X_R0E_BPDU_FR		BIT(25)
 #define  MT753X_R0E_EG_TAG_MASK		GENMASK(24, 22)
 #define  MT753X_R0E_EG_TAG(x)		FIELD_PREP(MT753X_R0E_EG_TAG_MASK, x)
 #define  MT753X_R0E_PORT_FW_MASK	GENMASK(18, 16)
 #define  MT753X_R0E_PORT_FW(x)		FIELD_PREP(MT753X_R0E_PORT_FW_MASK, x)
+#define  MT753X_R03_BPDU_FR		BIT(9)
 #define  MT753X_R03_EG_TAG_MASK		GENMASK(8, 6)
 #define  MT753X_R03_EG_TAG(x)		FIELD_PREP(MT753X_R03_EG_TAG_MASK, x)
 #define  MT753X_R03_PORT_FW_MASK	GENMASK(2, 0)

---
base-commit: 365af7ace014ef3fc6f5d0a373c96cc7193db4ce
change-id: 20240401-b4-for-net-mt7530-fix-link-local-when-stp-discarding-6e2a4e3e867a

Best regards,
-- 
Arınç ÜNAL <arinc.unal@arinc9.com>



_______________________________________________
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] arm64: dts: rockchip: Designate the system power controller on QuartzPro64
From: Dragan Simic @ 2024-04-07 17:56 UTC (permalink / raw)
  To: linux-rockchip
  Cc: heiko, linux-arm-kernel, devicetree, robh+dt,
	krzysztof.kozlowski+dt, conor+dt

Designate the primary RK806 PMIC on the Pine64 QuartzPro64 as the system
power controller, so the board shuts down properly on poweroff(8).

Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
index 67414d72e2b6..22bbfbe729c1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
@@ -456,6 +456,7 @@ pmic@0 {
 			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
 		pinctrl-names = "default";
 		spi-max-frequency = <1000000>;
+		system-power-controller;
 
 		vcc1-supply = <&vcc4v0_sys>;
 		vcc2-supply = <&vcc4v0_sys>;

_______________________________________________
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] arm64: dts: rockchip: mark system power controller and fix typo on orangepi-5-plus
From: Dragan Simic @ 2024-04-07 17:46 UTC (permalink / raw)
  To: efectn
  Cc: heiko, linux-rockchip, devicetree, linux-arm-kernel, linux-kernel,
	robh+dt, krzysztof.kozlowski+dt, conor+dt, sebastian.reichel,
	Muhammed Efe Cetin
In-Reply-To: <20240407173210.372585-1-efectn@6tel.net>

On 2024-04-07 19:32, efectn@6tel.net wrote:
> From: Muhammed Efe Cetin <efectn@protonmail.com>
> 
> Mark the PMIC as system power controller, so the board will shut-down
> properly and fix the typo on rk806_dvs1_null pins property.
> 
> Fixes: 236d225e1ee7 ("arm64: dts: rockchip: Add board device tree for
> rk3588-orangepi-5-plus")
> Signed-off-by: Muhammed Efe Cetin <efectn@protonmail.com>

Looking good to me.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> index 1b606ea5b6cf..1a604429fb26 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
> @@ -485,6 +485,7 @@ pmic@0 {
>  		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
>  			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
>  		spi-max-frequency = <1000000>;
> +		system-power-controller;
> 
>  		vcc1-supply = <&vcc5v0_sys>;
>  		vcc2-supply = <&vcc5v0_sys>;
> @@ -506,7 +507,7 @@ pmic@0 {
>  		#gpio-cells = <2>;
> 
>  		rk806_dvs1_null: dvs1-null-pins {
> -			pins = "gpio_pwrctrl2";
> +			pins = "gpio_pwrctrl1";
>  			function = "pin_fun0";
>  		};

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

^ permalink raw reply

* [PATCH] arm64: dts: rockchip: mark system power controller and fix typo on orangepi-5-plus
From: efectn @ 2024-04-07 17:32 UTC (permalink / raw)
  To: heiko, linux-rockchip
  Cc: devicetree, linux-arm-kernel, linux-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, sebastian.reichel,
	Muhammed Efe Cetin

From: Muhammed Efe Cetin <efectn@protonmail.com>

Mark the PMIC as system power controller, so the board will shut-down
properly and fix the typo on rk806_dvs1_null pins property.

Fixes: 236d225e1ee7 ("arm64: dts: rockchip: Add board device tree for rk3588-orangepi-5-plus")
Signed-off-by: Muhammed Efe Cetin <efectn@protonmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
index 1b606ea5b6cf..1a604429fb26 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-orangepi-5-plus.dts
@@ -485,6 +485,7 @@ pmic@0 {
 		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
 			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
 		spi-max-frequency = <1000000>;
+		system-power-controller;
 
 		vcc1-supply = <&vcc5v0_sys>;
 		vcc2-supply = <&vcc5v0_sys>;
@@ -506,7 +507,7 @@ pmic@0 {
 		#gpio-cells = <2>;
 
 		rk806_dvs1_null: dvs1-null-pins {
-			pins = "gpio_pwrctrl2";
+			pins = "gpio_pwrctrl1";
 			function = "pin_fun0";
 		};
 
-- 
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 related

* Re: [PATCH v2 6/7] s390: mm: accelerate pagefault when badaccess
From: Heiko Carstens @ 2024-04-07 17:19 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: akpm, Russell King, Catalin Marinas, Will Deacon,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexander Gordeev,
	Gerald Schaefer, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	x86, linux-arm-kernel, linuxppc-dev, linux-riscv, linux-s390,
	surenb, linux-mm
In-Reply-To: <20240403083805.1818160-7-wangkefeng.wang@huawei.com>

On Wed, Apr 03, 2024 at 04:38:04PM +0800, Kefeng Wang wrote:
> The vm_flags of vma already checked under per-VMA lock, if it is a
> bad access, directly handle error, no need to retry with mmap_lock
> again. Since the page faut is handled under per-VMA lock, count it
> as a vma lock event with VMA_LOCK_SUCCESS.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  arch/s390/mm/fault.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index c421dd44ffbe..162ca2576fd4 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -325,7 +325,8 @@ static void do_exception(struct pt_regs *regs, int access)
>  		goto lock_mmap;
>  	if (!(vma->vm_flags & access)) {
>  		vma_end_read(vma);
> -		goto lock_mmap;
> +		count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> +		return handle_fault_error_nolock(regs, SEGV_ACCERR);

Reviewed-by: Heiko Carstens <hca@linux.ibm.com>

_______________________________________________
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 v2 0/7] arch/mm/fault: accelerate pagefault when badaccess
From: Heiko Carstens @ 2024-04-07 17:19 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: Andrew Morton, Russell King, Catalin Marinas, Will Deacon,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexander Gordeev,
	Gerald Schaefer, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
	x86, linux-arm-kernel, linuxppc-dev, linux-riscv, linux-s390,
	surenb, linux-mm
In-Reply-To: <4d45fffc-877a-4ad1-ad02-a33341e467e7@huawei.com>

On Sun, Apr 07, 2024 at 03:49:53PM +0800, Kefeng Wang wrote:
> On 2024/4/4 4:45, Andrew Morton wrote:
> > On Wed, 3 Apr 2024 16:37:58 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> > 
> > > After VMA lock-based page fault handling enabled, if bad access met
> > > under per-vma lock, it will fallback to mmap_lock-based handling,
> > > so it leads to unnessary mmap lock and vma find again. A test from
> > > lmbench shows 34% improve after this changes on arm64,
> > > 
> > >    lat_sig -P 1 prot lat_sig 0.29194 -> 0.19198
> > > 
> > > Only build test on other archs except arm64.
> > 
> > Thanks.  So we now want a bunch of architectures to runtime test this.  Do
> > we have a selftest in place which will adequately do this?
> 
> I don't find such selftest, and badaccess would lead to coredump, the
> performance should not affect most scene, so no selftest is acceptable.
> lmbench is easy to use to measure the performance.

The rationale for this series (performance improvement) is a bit odd,
since I would expect that the changed code is usually never executed.

_______________________________________________
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 net-next v1 00/12] First try to replace page_frag with page_frag_cache
From: Alexander Duyck @ 2024-04-07 17:02 UTC (permalink / raw)
  To: Yunsheng Lin
  Cc: davem, kuba, pabeni, netdev, linux-kernel, Matthias Brugger,
	AngeloGioacchino Del Regno, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, linux-arm-kernel,
	linux-mediatek, bpf
In-Reply-To: <20240407130850.19625-1-linyunsheng@huawei.com>

On Sun, Apr 7, 2024 at 6:10 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> After [1], Only there are two implementations for page frag:
>
> 1. mm/page_alloc.c: net stack seems to be using it in the
>    rx part with 'struct page_frag_cache' and the main API
>    being page_frag_alloc_align().
> 2. net/core/sock.c: net stack seems to be using it in the
>    tx part with 'struct page_frag' and the main API being
>    skb_page_frag_refill().
>
> This patchset tries to unfiy the page frag implementation
> by replacing page_frag with page_frag_cache for sk_page_frag()
> first. net_high_order_alloc_disable_key for the implementation
> in net/core/sock.c doesn't seems matter that much now have
> have pcp support for high-order pages in commit 44042b449872
> ("mm/page_alloc: allow high-order pages to be stored on the
> per-cpu lists").
>
> As the related change is mostly related to networking, so
> targeting the net-next. And will try to replace the rest
> of page_frag in the follow patchset.
>
> After this patchset, we are not only able to unify the page
> frag implementation a little, but seems able to have about
> 0.5+% performance boost testing by using the vhost_net_test
> introduced in [1] and page_frag_test.ko introduced in this
> patch.

One question that jumps out at me for this is "why?". No offense but
this is a pretty massive set of changes with over 1400 additions and
500+ deletions and I can't help but ask why, and this cover page
doesn't give me any good reason to think about accepting this set.
What is meant to be the benefit to the community for adding this? All
I am seeing is a ton of extra code to have to review as this
unification is adding an additional 1000+ lines without a good
explanation as to why they are needed.

Also I wouldn't bother mentioning the 0.5+% performance gain as a
"bonus". Changes of that amount usually mean it is within the margin
of error. At best it likely means you haven't introduced a noticeable
regression.

_______________________________________________
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 v4 0/5] dmaengine: fsl-sdma: Some improvement for fsl-sdma
From: Vinod Koul @ 2024-04-07 16:39 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Joy Zou, Frank Li
  Cc: dmaengine, linux-arm-kernel, linux-kernel, devicetree, imx,
	Nicolin Chen, Shengjiu Wang, Daniel Baluta, Vipul Kumar,
	Srikanth Krishnakar, Robin Gong, Iuliana Prodan, Clark Wang
In-Reply-To: <20240329-sdma_upstream-v4-0-daeb3067dea7@nxp.com>


On Fri, 29 Mar 2024 10:34:40 -0400, Frank Li wrote:
> To: Vinod Koul <vkoul@kernel.org>
> To: Shawn Guo <shawnguo@kernel.org>
> To: Sascha Hauer <s.hauer@pengutronix.de>
> To: Pengutronix Kernel Team <kernel@pengutronix.de>
> To: Fabio Estevam <festevam@gmail.com>
> To: NXP Linux Team <linux-imx@nxp.com>
> To: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Joy Zou <joy.zou@nxp.com>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: imx@lists.linux.dev
> 
> [...]

Applied, thanks!

[1/5] dmaengine: imx-sdma: Support allocate memory from internal SRAM (iram)
      commit: 802ef223101fec83d92e045f89000b228904a580
[2/5] dmaengine: imx-sdma: Support 24bit/3bytes for sg mode
      commit: 288109387becd8abadca5c063c70a07ae0dd7716
[3/5] dmaengine: imx-sdma: support dual fifo for DEV_TO_DEV
      commit: a20f10d6accb9f5096fa7a7296e5ae34f4562440
[4/5] dt-bindings: fsl-imx-sdma: Add I2C peripheral types ID
      (no commit info)
[5/5] dmaengine: imx-sdma: Add i2c dma support
      (no commit info)

Best regards,
-- 
~Vinod



_______________________________________________
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 0/3] dmaengine: xilinx: xdma: Various fixes for xdma
From: Vinod Koul @ 2024-04-07 16:38 UTC (permalink / raw)
  To: Lizhi Hou, Brian Xu, Raj Kumar Rampelli, Michal Simek, Jan Kuliga,
	Miquel Raynal, Louis Chauvet
  Cc: dmaengine, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <20240327-digigram-xdma-fixes-v1-0-45f4a52c0283@bootlin.com>


On Wed, 27 Mar 2024 10:58:47 +0100, Louis Chauvet wrote:
> The current driver have some issues, this series fixes them.
> 
> PATCH 1 is fixing a wrong offset computation in the dma descriptor address
> PATCH 2 is fixing the xdma_synchronize callback, which was not waiting
>    properly for the last transfer.
> PATCH 3 is clarifing the documentation for xdma_fill_descs
> 
> [...]

Applied, thanks!

[1/3] dmaengine: xilinx: xdma: Fix wrong offsets in the buffers addresses in dma descriptor
      commit: 5b9706bfc094314c600ab810a61208a7cbaa4cb3
[2/3] dmaengine: xilinx: xdma: Fix synchronization issue
      commit: 6a40fb8245965b481b4dcce011cd63f20bf91ee0
[3/3] dmaengine: xilinx: xdma: Clarify kdoc in XDMA driver
      commit: 7a71c6dc21d5ae83ab27c39a67845d6d23ac271f

Best regards,
-- 
~Vinod



_______________________________________________
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 0/3] dma: xilinx_dpdma: Fix locking
From: Vinod Koul @ 2024-04-07 16:38 UTC (permalink / raw)
  To: Laurent Pinchart, dmaengine, Sean Anderson
  Cc: Michal Simek, linux-kernel, linux-arm-kernel, Hyun Kwon,
	Tejas Upadhyay
In-Reply-To: <20240308210034.3634938-1-sean.anderson@linux.dev>


On Fri, 08 Mar 2024 16:00:31 -0500, Sean Anderson wrote:
> This series fixes some locking problems with the xilinx dpdma driver. It
> also adds some additional lockdep asserts to make catching such errors
> easier.
> 
> 
> Sean Anderson (3):
>   dma: xilinx_dpdma: Fix locking
>   dma: xilinx_dpdma: Remove unnecessary use of irqsave/restore
>   dma: Add lockdep asserts to virt-dma
> 
> [...]

Applied, thanks!

[1/3] dma: xilinx_dpdma: Fix locking
      commit: 244296cc3a155199a8b080d19e645d7d49081a38
[2/3] dma: xilinx_dpdma: Remove unnecessary use of irqsave/restore
      (no commit info)
[3/3] dma: Add lockdep asserts to virt-dma
      (no commit info)

Best regards,
-- 
~Vinod



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

^ permalink raw reply

* [PATCH v9 6/7] spmi: pmic-arb: Register controller for bus instead of arbiter
From: Abel Vesa @ 2024-04-07 16:23 UTC (permalink / raw)
  To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Srini Kandagatla, Johan Hovold, David Collins, linux-kernel,
	linux-arm-kernel, linux-arm-msm, linux-mediatek, devicetree,
	Abel Vesa
In-Reply-To: <20240407-spmi-multi-master-support-v9-0-fa151c1391f3@linaro.org>

Introduce the bus object in order to decouple the resources
that are bus specific from the arbiter. This way the SPMI controller
is registered with the generic framework at a bus level rather than
arbiter. This is needed in order to prepare for multi bus support.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/spmi/spmi-pmic-arb.c | 659 ++++++++++++++++++++++++-------------------
 1 file changed, 374 insertions(+), 285 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index ff777b4a6f33..65d8f45da24b 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spmi.h>
@@ -125,61 +126,72 @@ struct apid_data {
 	u8		irq_ee;
 };
 
+struct spmi_pmic_arb;
+
 /**
- * struct spmi_pmic_arb - SPMI PMIC Arbiter object
+ * struct spmi_pmic_arb_bus - SPMI PMIC Arbiter Bus object
  *
- * @rd_base:		on v1 "core", on v2 "observer" register base off DT.
- * @wr_base:		on v1 "core", on v2 "chnls"    register base off DT.
+ * @pmic_arb:		the SPMI PMIC Arbiter the bus belongs to.
+ * @domain:		irq domain object for PMIC IRQ domain
  * @intr:		address of the SPMI interrupt control registers.
  * @cnfg:		address of the PMIC Arbiter configuration registers.
- * @core:		core register base for v2 and above only (see above)
- * @core_size:		core register base size
+ * @spmic:		spmi controller registered for this bus
  * @lock:		lock to synchronize accesses.
- * @channel:		execution environment channel to use for accesses.
- * @irq:		PMIC ARB interrupt.
- * @ee:			the current Execution Environment
- * @bus_instance:	on v7: 0 = primary SPMI bus, 1 = secondary SPMI bus
- * @min_apid:		minimum APID (used for bounding IRQ search)
- * @max_apid:		maximum APID
  * @base_apid:		on v7: minimum APID associated with the particular SPMI
  *			bus instance
  * @apid_count:		on v5 and v7: number of APIDs associated with the
  *			particular SPMI bus instance
  * @mapping_table:	in-memory copy of PPID -> APID mapping table.
  * @mapping_table_valid:bitmap containing valid-only periphs
- * @domain:		irq domain object for PMIC IRQ domain
- * @spmic:		SPMI controller object
- * @ver_ops:		version dependent operations.
  * @ppid_to_apid:	in-memory copy of PPID -> APID mapping table.
  * @last_apid:		Highest value APID in use
  * @apid_data:		Table of data for all APIDs
- * @max_periphs:	Number of elements in apid_data[]
+ * @min_apid:		minimum APID (used for bounding IRQ search)
+ * @max_apid:		maximum APID
+ * @irq:		PMIC ARB interrupt.
  */
-struct spmi_pmic_arb {
-	void __iomem		*rd_base;
-	void __iomem		*wr_base;
+struct spmi_pmic_arb_bus {
+	struct spmi_pmic_arb	*pmic_arb;
+	struct irq_domain	*domain;
 	void __iomem		*intr;
 	void __iomem		*cnfg;
-	void __iomem		*core;
-	resource_size_t		core_size;
+	struct spmi_controller	*spmic;
 	raw_spinlock_t		lock;
-	u8			channel;
-	int			irq;
-	u8			ee;
-	u32			bus_instance;
-	u16			min_apid;
-	u16			max_apid;
 	u16			base_apid;
 	int			apid_count;
 	u32			*mapping_table;
 	DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
-	struct irq_domain	*domain;
-	struct spmi_controller	*spmic;
-	const struct pmic_arb_ver_ops *ver_ops;
 	u16			*ppid_to_apid;
 	u16			last_apid;
 	struct apid_data	*apid_data;
+	u16			min_apid;
+	u16			max_apid;
+	int			irq;
+};
+
+/**
+ * struct spmi_pmic_arb - SPMI PMIC Arbiter object
+ *
+ * @rd_base:		on v1 "core", on v2 "observer" register base off DT.
+ * @wr_base:		on v1 "core", on v2 "chnls"    register base off DT.
+ * @core:		core register base for v2 and above only (see above)
+ * @core_size:		core register base size
+ * @channel:		execution environment channel to use for accesses.
+ * @ee:			the current Execution Environment
+ * @ver_ops:		version dependent operations.
+ * @max_periphs:	Number of elements in apid_data[]
+ * @bus:		per arbiter bus instance
+ */
+struct spmi_pmic_arb {
+	void __iomem		*rd_base;
+	void __iomem		*wr_base;
+	void __iomem		*core;
+	resource_size_t		core_size;
+	u8			channel;
+	u8			ee;
+	const struct pmic_arb_ver_ops *ver_ops;
 	int			max_periphs;
+	struct spmi_pmic_arb_bus *bus;
 };
 
 /**
@@ -208,21 +220,21 @@ struct spmi_pmic_arb {
 struct pmic_arb_ver_ops {
 	const char *ver_str;
 	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
-	int (*init_apid)(struct spmi_pmic_arb *pmic_arb);
-	int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
+	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
+	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
 	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
-	int (*offset)(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
-			enum pmic_arb_channel ch_type);
+	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
+		      enum pmic_arb_channel ch_type);
 	u32 (*fmt_cmd)(u8 opc, u8 sid, u16 addr, u8 bc);
 	int (*non_data_cmd)(struct spmi_controller *ctrl, u8 opc, u8 sid);
 	/* Interrupts controller functionality (offset of PIC registers) */
-	void __iomem *(*owner_acc_status)(struct spmi_pmic_arb *pmic_arb, u8 m,
+	void __iomem *(*owner_acc_status)(struct spmi_pmic_arb_bus *bus, u8 m,
 					  u16 n);
-	void __iomem *(*acc_enable)(struct spmi_pmic_arb *pmic_arb, u16 n);
-	void __iomem *(*irq_status)(struct spmi_pmic_arb *pmic_arb, u16 n);
-	void __iomem *(*irq_clear)(struct spmi_pmic_arb *pmic_arb, u16 n);
+	void __iomem *(*acc_enable)(struct spmi_pmic_arb_bus *bus, u16 n);
+	void __iomem *(*irq_status)(struct spmi_pmic_arb_bus *bus, u16 n);
+	void __iomem *(*irq_clear)(struct spmi_pmic_arb_bus *bus, u16 n);
 	u32 (*apid_map_offset)(u16 n);
-	void __iomem *(*apid_owner)(struct spmi_pmic_arb *pmic_arb, u16 n);
+	void __iomem *(*apid_owner)(struct spmi_pmic_arb_bus *bus, u16 n);
 };
 
 static inline void pmic_arb_base_write(struct spmi_pmic_arb *pmic_arb,
@@ -272,13 +284,14 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 				  void __iomem *base, u8 sid, u16 addr,
 				  enum pmic_arb_channel ch_type)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u32 status = 0;
 	u32 timeout = PMIC_ARB_TIMEOUT_US;
 	u32 offset;
 	int rc;
 
-	rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr, ch_type);
+	rc = pmic_arb->ver_ops->offset(bus, sid, addr, ch_type);
 	if (rc < 0)
 		return rc;
 
@@ -321,24 +334,25 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 static int
 pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	unsigned long flags;
 	u32 cmd;
 	int rc;
 	u32 offset;
 
-	rc = pmic_arb->ver_ops->offset(pmic_arb, sid, 0, PMIC_ARB_CHANNEL_RW);
+	rc = pmic_arb->ver_ops->offset(bus, sid, 0, PMIC_ARB_CHANNEL_RW);
 	if (rc < 0)
 		return rc;
 
 	offset = rc;
 	cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
 
-	raw_spin_lock_irqsave(&pmic_arb->lock, flags);
+	raw_spin_lock_irqsave(&bus->lock, flags);
 	pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
 	rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, 0,
 				    PMIC_ARB_CHANNEL_RW);
-	raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	raw_spin_unlock_irqrestore(&bus->lock, flags);
 
 	return rc;
 }
@@ -363,20 +377,21 @@ static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
 	return pmic_arb->ver_ops->non_data_cmd(ctrl, opc, sid);
 }
 
-static int pmic_arb_fmt_read_cmd(struct spmi_pmic_arb *pmic_arb, u8 opc, u8 sid,
+static int pmic_arb_fmt_read_cmd(struct spmi_pmic_arb_bus *bus, u8 opc, u8 sid,
 				 u16 addr, size_t len, u32 *cmd, u32 *offset)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u8 bc = len - 1;
 	int rc;
 
-	rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr,
+	rc = pmic_arb->ver_ops->offset(bus, sid, addr,
 				       PMIC_ARB_CHANNEL_OBS);
 	if (rc < 0)
 		return rc;
 
 	*offset = rc;
 	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
-		dev_err(&pmic_arb->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
+		dev_err(&bus->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
 			PMIC_ARB_MAX_TRANS_BYTES, len);
 		return  -EINVAL;
 	}
@@ -400,7 +415,8 @@ static int pmic_arb_read_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
 				      u32 offset, u8 sid, u16 addr, u8 *buf,
 				      size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u8 bc = len - 1;
 	int rc;
 
@@ -422,38 +438,39 @@ static int pmic_arb_read_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
 static int pmic_arb_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
 			     u16 addr, u8 *buf, size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
 	unsigned long flags;
 	u32 cmd, offset;
 	int rc;
 
-	rc = pmic_arb_fmt_read_cmd(pmic_arb, opc, sid, addr, len, &cmd,
+	rc = pmic_arb_fmt_read_cmd(bus, opc, sid, addr, len, &cmd,
 				   &offset);
 	if (rc)
 		return rc;
 
-	raw_spin_lock_irqsave(&pmic_arb->lock, flags);
+	raw_spin_lock_irqsave(&bus->lock, flags);
 	rc = pmic_arb_read_cmd_unlocked(ctrl, cmd, offset, sid, addr, buf, len);
-	raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	raw_spin_unlock_irqrestore(&bus->lock, flags);
 
 	return rc;
 }
 
-static int pmic_arb_fmt_write_cmd(struct spmi_pmic_arb *pmic_arb, u8 opc,
+static int pmic_arb_fmt_write_cmd(struct spmi_pmic_arb_bus *bus, u8 opc,
 				  u8 sid, u16 addr, size_t len, u32 *cmd,
 				  u32 *offset)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u8 bc = len - 1;
 	int rc;
 
-	rc = pmic_arb->ver_ops->offset(pmic_arb, sid, addr,
+	rc = pmic_arb->ver_ops->offset(bus, sid, addr,
 					PMIC_ARB_CHANNEL_RW);
 	if (rc < 0)
 		return rc;
 
 	*offset = rc;
 	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
-		dev_err(&pmic_arb->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
+		dev_err(&bus->spmic->dev, "pmic-arb supports 1..%d bytes per trans, but:%zu requested",
 			PMIC_ARB_MAX_TRANS_BYTES, len);
 		return  -EINVAL;
 	}
@@ -479,7 +496,8 @@ static int pmic_arb_write_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
 				      u32 offset, u8 sid, u16 addr,
 				      const u8 *buf, size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u8 bc = len - 1;
 
 	/* Write data to FIFOs */
@@ -498,20 +516,20 @@ static int pmic_arb_write_cmd_unlocked(struct spmi_controller *ctrl, u32 cmd,
 static int pmic_arb_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
 			      u16 addr, const u8 *buf, size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
 	unsigned long flags;
 	u32 cmd, offset;
 	int rc;
 
-	rc = pmic_arb_fmt_write_cmd(pmic_arb, opc, sid, addr, len, &cmd,
+	rc = pmic_arb_fmt_write_cmd(bus, opc, sid, addr, len, &cmd,
 				    &offset);
 	if (rc)
 		return rc;
 
-	raw_spin_lock_irqsave(&pmic_arb->lock, flags);
+	raw_spin_lock_irqsave(&bus->lock, flags);
 	rc = pmic_arb_write_cmd_unlocked(ctrl, cmd, offset, sid, addr, buf,
 					 len);
-	raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	raw_spin_unlock_irqrestore(&bus->lock, flags);
 
 	return rc;
 }
@@ -519,23 +537,23 @@ static int pmic_arb_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
 static int pmic_arb_masked_write(struct spmi_controller *ctrl, u8 sid, u16 addr,
 				 const u8 *buf, const u8 *mask, size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	struct spmi_pmic_arb_bus *bus = spmi_controller_get_drvdata(ctrl);
 	u32 read_cmd, read_offset, write_cmd, write_offset;
 	u8 temp[PMIC_ARB_MAX_TRANS_BYTES];
 	unsigned long flags;
 	int rc, i;
 
-	rc = pmic_arb_fmt_read_cmd(pmic_arb, SPMI_CMD_EXT_READL, sid, addr, len,
+	rc = pmic_arb_fmt_read_cmd(bus, SPMI_CMD_EXT_READL, sid, addr, len,
 				   &read_cmd, &read_offset);
 	if (rc)
 		return rc;
 
-	rc = pmic_arb_fmt_write_cmd(pmic_arb, SPMI_CMD_EXT_WRITEL, sid, addr,
+	rc = pmic_arb_fmt_write_cmd(bus, SPMI_CMD_EXT_WRITEL, sid, addr,
 				    len, &write_cmd, &write_offset);
 	if (rc)
 		return rc;
 
-	raw_spin_lock_irqsave(&pmic_arb->lock, flags);
+	raw_spin_lock_irqsave(&bus->lock, flags);
 	rc = pmic_arb_read_cmd_unlocked(ctrl, read_cmd, read_offset, sid, addr,
 					temp, len);
 	if (rc)
@@ -547,7 +565,7 @@ static int pmic_arb_masked_write(struct spmi_controller *ctrl, u8 sid, u16 addr,
 	rc = pmic_arb_write_cmd_unlocked(ctrl, write_cmd, write_offset, sid,
 					 addr, temp, len);
 done:
-	raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	raw_spin_unlock_irqrestore(&bus->lock, flags);
 
 	return rc;
 }
@@ -573,25 +591,25 @@ struct spmi_pmic_arb_qpnpint_type {
 static void qpnpint_spmi_write(struct irq_data *d, u8 reg, void *buf,
 			       size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
 	u8 sid = hwirq_to_sid(d->hwirq);
 	u8 per = hwirq_to_per(d->hwirq);
 
-	if (pmic_arb_write_cmd(pmic_arb->spmic, SPMI_CMD_EXT_WRITEL, sid,
+	if (pmic_arb_write_cmd(bus->spmic, SPMI_CMD_EXT_WRITEL, sid,
 			       (per << 8) + reg, buf, len))
-		dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x\n",
+		dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x\n",
 				    d->irq);
 }
 
 static void qpnpint_spmi_read(struct irq_data *d, u8 reg, void *buf, size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
 	u8 sid = hwirq_to_sid(d->hwirq);
 	u8 per = hwirq_to_per(d->hwirq);
 
-	if (pmic_arb_read_cmd(pmic_arb->spmic, SPMI_CMD_EXT_READL, sid,
+	if (pmic_arb_read_cmd(bus->spmic, SPMI_CMD_EXT_READL, sid,
 			      (per << 8) + reg, buf, len))
-		dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x\n",
+		dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x\n",
 				    d->irq);
 }
 
@@ -599,47 +617,49 @@ static int qpnpint_spmi_masked_write(struct irq_data *d, u8 reg,
 				     const void *buf, const void *mask,
 				     size_t len)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
 	u8 sid = hwirq_to_sid(d->hwirq);
 	u8 per = hwirq_to_per(d->hwirq);
 	int rc;
 
-	rc = pmic_arb_masked_write(pmic_arb->spmic, sid, (per << 8) + reg, buf,
+	rc = pmic_arb_masked_write(bus->spmic, sid, (per << 8) + reg, buf,
 				   mask, len);
 	if (rc)
-		dev_err_ratelimited(&pmic_arb->spmic->dev, "failed irqchip transaction on %x rc=%d\n",
+		dev_err_ratelimited(&bus->spmic->dev, "failed irqchip transaction on %x rc=%d\n",
 				    d->irq, rc);
 	return rc;
 }
 
-static void cleanup_irq(struct spmi_pmic_arb *pmic_arb, u16 apid, int id)
+static void cleanup_irq(struct spmi_pmic_arb_bus *bus, u16 apid, int id)
 {
-	u16 ppid = pmic_arb->apid_data[apid].ppid;
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
+	u16 ppid = bus->apid_data[apid].ppid;
 	u8 sid = ppid >> 8;
 	u8 per = ppid & 0xFF;
 	u8 irq_mask = BIT(id);
 
-	dev_err_ratelimited(&pmic_arb->spmic->dev, "%s apid=%d sid=0x%x per=0x%x irq=%d\n",
-			__func__, apid, sid, per, id);
-	writel_relaxed(irq_mask, pmic_arb->ver_ops->irq_clear(pmic_arb, apid));
+	dev_err_ratelimited(&bus->spmic->dev, "%s apid=%d sid=0x%x per=0x%x irq=%d\n",
+			    __func__, apid, sid, per, id);
+	writel_relaxed(irq_mask, pmic_arb->ver_ops->irq_clear(bus, apid));
 }
 
-static int periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
+static int periph_interrupt(struct spmi_pmic_arb_bus *bus, u16 apid)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	unsigned int irq;
 	u32 status, id;
 	int handled = 0;
-	u8 sid = (pmic_arb->apid_data[apid].ppid >> 8) & 0xF;
-	u8 per = pmic_arb->apid_data[apid].ppid & 0xFF;
+	u8 sid = (bus->apid_data[apid].ppid >> 8) & 0xF;
+	u8 per = bus->apid_data[apid].ppid & 0xFF;
 
-	status = readl_relaxed(pmic_arb->ver_ops->irq_status(pmic_arb, apid));
+	status = readl_relaxed(pmic_arb->ver_ops->irq_status(bus, apid));
 	while (status) {
 		id = ffs(status) - 1;
 		status &= ~BIT(id);
-		irq = irq_find_mapping(pmic_arb->domain,
-					spec_to_hwirq(sid, per, id, apid));
+		irq = irq_find_mapping(bus->domain,
+				       spec_to_hwirq(sid, per, id, apid));
 		if (irq == 0) {
-			cleanup_irq(pmic_arb, apid, id);
+			cleanup_irq(bus, apid, id);
 			continue;
 		}
 		generic_handle_irq(irq);
@@ -651,16 +671,17 @@ static int periph_interrupt(struct spmi_pmic_arb *pmic_arb, u16 apid)
 
 static void pmic_arb_chained_irq(struct irq_desc *desc)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_desc_get_handler_data(desc);
+	struct spmi_pmic_arb_bus *bus = irq_desc_get_handler_data(desc);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	const struct pmic_arb_ver_ops *ver_ops = pmic_arb->ver_ops;
 	struct irq_chip *chip = irq_desc_get_chip(desc);
-	int first = pmic_arb->min_apid;
-	int last = pmic_arb->max_apid;
+	int first = bus->min_apid;
+	int last = bus->max_apid;
 	/*
 	 * acc_offset will be non-zero for the secondary SPMI bus instance on
 	 * v7 controllers.
 	 */
-	int acc_offset = pmic_arb->base_apid >> 5;
+	int acc_offset = bus->base_apid >> 5;
 	u8 ee = pmic_arb->ee;
 	u32 status, enable, handled = 0;
 	int i, id, apid;
@@ -671,7 +692,7 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 	chained_irq_enter(chip, desc);
 
 	for (i = first >> 5; i <= last >> 5; ++i) {
-		status = readl_relaxed(ver_ops->owner_acc_status(pmic_arb, ee, i - acc_offset));
+		status = readl_relaxed(ver_ops->owner_acc_status(bus, ee, i - acc_offset));
 		if (status)
 			acc_valid = true;
 
@@ -685,9 +706,9 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 				continue;
 			}
 			enable = readl_relaxed(
-					ver_ops->acc_enable(pmic_arb, apid));
+					ver_ops->acc_enable(bus, apid));
 			if (enable & SPMI_PIC_ACC_ENABLE_BIT)
-				if (periph_interrupt(pmic_arb, apid) != 0)
+				if (periph_interrupt(bus, apid) != 0)
 					handled++;
 		}
 	}
@@ -696,19 +717,19 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 	if (!acc_valid) {
 		for (i = first; i <= last; i++) {
 			/* skip if APPS is not irq owner */
-			if (pmic_arb->apid_data[i].irq_ee != pmic_arb->ee)
+			if (bus->apid_data[i].irq_ee != pmic_arb->ee)
 				continue;
 
 			irq_status = readl_relaxed(
-					     ver_ops->irq_status(pmic_arb, i));
+					     ver_ops->irq_status(bus, i));
 			if (irq_status) {
 				enable = readl_relaxed(
-					     ver_ops->acc_enable(pmic_arb, i));
+					     ver_ops->acc_enable(bus, i));
 				if (enable & SPMI_PIC_ACC_ENABLE_BIT) {
-					dev_dbg(&pmic_arb->spmic->dev,
+					dev_dbg(&bus->spmic->dev,
 						"Dispatching IRQ for apid=%d status=%x\n",
 						i, irq_status);
-					if (periph_interrupt(pmic_arb, i) != 0)
+					if (periph_interrupt(bus, i) != 0)
 						handled++;
 				}
 			}
@@ -723,12 +744,13 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 
 static void qpnpint_irq_ack(struct irq_data *d)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u8 irq = hwirq_to_irq(d->hwirq);
 	u16 apid = hwirq_to_apid(d->hwirq);
 	u8 data;
 
-	writel_relaxed(BIT(irq), pmic_arb->ver_ops->irq_clear(pmic_arb, apid));
+	writel_relaxed(BIT(irq), pmic_arb->ver_ops->irq_clear(bus, apid));
 
 	data = BIT(irq);
 	qpnpint_spmi_write(d, QPNPINT_REG_LATCHED_CLR, &data, 1);
@@ -744,14 +766,15 @@ static void qpnpint_irq_mask(struct irq_data *d)
 
 static void qpnpint_irq_unmask(struct irq_data *d)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	const struct pmic_arb_ver_ops *ver_ops = pmic_arb->ver_ops;
 	u8 irq = hwirq_to_irq(d->hwirq);
 	u16 apid = hwirq_to_apid(d->hwirq);
 	u8 buf[2];
 
 	writel_relaxed(SPMI_PIC_ACC_ENABLE_BIT,
-			ver_ops->acc_enable(pmic_arb, apid));
+			ver_ops->acc_enable(bus, apid));
 
 	qpnpint_spmi_read(d, QPNPINT_REG_EN_SET, &buf[0], 1);
 	if (!(buf[0] & BIT(irq))) {
@@ -808,9 +831,9 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
 
 static int qpnpint_irq_set_wake(struct irq_data *d, unsigned int on)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
 
-	return irq_set_irq_wake(pmic_arb->irq, on);
+	return irq_set_irq_wake(bus->irq, on);
 }
 
 static int qpnpint_get_irqchip_state(struct irq_data *d,
@@ -832,17 +855,18 @@ static int qpnpint_get_irqchip_state(struct irq_data *d,
 static int qpnpint_irq_domain_activate(struct irq_domain *domain,
 				       struct irq_data *d, bool reserve)
 {
-	struct spmi_pmic_arb *pmic_arb = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb_bus *bus = irq_data_get_irq_chip_data(d);
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u16 periph = hwirq_to_per(d->hwirq);
 	u16 apid = hwirq_to_apid(d->hwirq);
 	u16 sid = hwirq_to_sid(d->hwirq);
 	u16 irq = hwirq_to_irq(d->hwirq);
 	u8 buf;
 
-	if (pmic_arb->apid_data[apid].irq_ee != pmic_arb->ee) {
-		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u: ee=%u but owner=%u\n",
+	if (bus->apid_data[apid].irq_ee != pmic_arb->ee) {
+		dev_err(&bus->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u: ee=%u but owner=%u\n",
 			sid, periph, irq, pmic_arb->ee,
-			pmic_arb->apid_data[apid].irq_ee);
+			bus->apid_data[apid].irq_ee);
 		return -ENODEV;
 	}
 
@@ -869,15 +893,16 @@ static int qpnpint_irq_domain_translate(struct irq_domain *d,
 					unsigned long *out_hwirq,
 					unsigned int *out_type)
 {
-	struct spmi_pmic_arb *pmic_arb = d->host_data;
+	struct spmi_pmic_arb_bus *bus = d->host_data;
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u32 *intspec = fwspec->param;
 	u16 apid, ppid;
 	int rc;
 
-	dev_dbg(&pmic_arb->spmic->dev, "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
+	dev_dbg(&bus->spmic->dev, "intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
 		intspec[0], intspec[1], intspec[2]);
 
-	if (irq_domain_get_of_node(d) != pmic_arb->spmic->dev.of_node)
+	if (irq_domain_get_of_node(d) != bus->spmic->dev.of_node)
 		return -EINVAL;
 	if (fwspec->param_count != 4)
 		return -EINVAL;
@@ -885,37 +910,37 @@ static int qpnpint_irq_domain_translate(struct irq_domain *d,
 		return -EINVAL;
 
 	ppid = intspec[0] << 8 | intspec[1];
-	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
+	rc = pmic_arb->ver_ops->ppid_to_apid(bus, ppid);
 	if (rc < 0) {
-		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
-		intspec[0], intspec[1], intspec[2], rc);
+		dev_err(&bus->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
+			intspec[0], intspec[1], intspec[2], rc);
 		return rc;
 	}
 
 	apid = rc;
 	/* Keep track of {max,min}_apid for bounding search during interrupt */
-	if (apid > pmic_arb->max_apid)
-		pmic_arb->max_apid = apid;
-	if (apid < pmic_arb->min_apid)
-		pmic_arb->min_apid = apid;
+	if (apid > bus->max_apid)
+		bus->max_apid = apid;
+	if (apid < bus->min_apid)
+		bus->min_apid = apid;
 
 	*out_hwirq = spec_to_hwirq(intspec[0], intspec[1], intspec[2], apid);
 	*out_type  = intspec[3] & IRQ_TYPE_SENSE_MASK;
 
-	dev_dbg(&pmic_arb->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
+	dev_dbg(&bus->spmic->dev, "out_hwirq = %lu\n", *out_hwirq);
 
 	return 0;
 }
 
 static struct lock_class_key qpnpint_irq_lock_class, qpnpint_irq_request_class;
 
-static void qpnpint_irq_domain_map(struct spmi_pmic_arb *pmic_arb,
+static void qpnpint_irq_domain_map(struct spmi_pmic_arb_bus *bus,
 				   struct irq_domain *domain, unsigned int virq,
 				   irq_hw_number_t hwirq, unsigned int type)
 {
 	irq_flow_handler_t handler;
 
-	dev_dbg(&pmic_arb->spmic->dev, "virq = %u, hwirq = %lu, type = %u\n",
+	dev_dbg(&bus->spmic->dev, "virq = %u, hwirq = %lu, type = %u\n",
 		virq, hwirq, type);
 
 	if (type & IRQ_TYPE_EDGE_BOTH)
@@ -926,7 +951,7 @@ static void qpnpint_irq_domain_map(struct spmi_pmic_arb *pmic_arb,
 
 	irq_set_lockdep_class(virq, &qpnpint_irq_lock_class,
 			      &qpnpint_irq_request_class);
-	irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, pmic_arb,
+	irq_domain_set_info(domain, virq, hwirq, &pmic_arb_irqchip, bus,
 			    handler, NULL, NULL);
 }
 
@@ -934,7 +959,7 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
 				    unsigned int virq, unsigned int nr_irqs,
 				    void *data)
 {
-	struct spmi_pmic_arb *pmic_arb = domain->host_data;
+	struct spmi_pmic_arb_bus *bus = domain->host_data;
 	struct irq_fwspec *fwspec = data;
 	irq_hw_number_t hwirq;
 	unsigned int type;
@@ -945,20 +970,22 @@ static int qpnpint_irq_domain_alloc(struct irq_domain *domain,
 		return ret;
 
 	for (i = 0; i < nr_irqs; i++)
-		qpnpint_irq_domain_map(pmic_arb, domain, virq + i, hwirq + i,
+		qpnpint_irq_domain_map(bus, domain, virq + i, hwirq + i,
 				       type);
 
 	return 0;
 }
 
-static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
+static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb_bus *bus)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
+
 	/*
 	 * Initialize max_apid/min_apid to the opposite bounds, during
 	 * the irq domain translation, we are sure to update these
 	 */
-	pmic_arb->max_apid = 0;
-	pmic_arb->min_apid = pmic_arb->max_periphs - 1;
+	bus->max_apid = 0;
+	bus->min_apid = pmic_arb->max_periphs - 1;
 
 	return 0;
 }
@@ -976,37 +1003,38 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
 	return 0;
 }
 
-static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb)
+static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u32 *mapping_table;
 
-	mapping_table = devm_kcalloc(&pmic_arb->spmic->dev, pmic_arb->max_periphs,
+	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
 				     sizeof(*mapping_table), GFP_KERNEL);
 	if (!mapping_table)
 		return -ENOMEM;
 
-	pmic_arb->mapping_table = mapping_table;
+	bus->mapping_table = mapping_table;
 
-	return pmic_arb_init_apid_min_max(pmic_arb);
+	return pmic_arb_init_apid_min_max(bus);
 }
 
-static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
+static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb_bus *bus, u16 ppid)
 {
-	u32 *mapping_table = pmic_arb->mapping_table;
+	u32 *mapping_table = bus->mapping_table;
 	int index = 0, i;
 	u16 apid_valid;
 	u16 apid;
 	u32 data;
 
-	apid_valid = pmic_arb->ppid_to_apid[ppid];
+	apid_valid = bus->ppid_to_apid[ppid];
 	if (apid_valid & PMIC_ARB_APID_VALID) {
 		apid = apid_valid & ~PMIC_ARB_APID_VALID;
 		return apid;
 	}
 
 	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
-		if (!test_and_set_bit(index, pmic_arb->mapping_table_valid))
-			mapping_table[index] = readl_relaxed(pmic_arb->cnfg +
+		if (!test_and_set_bit(index, bus->mapping_table_valid))
+			mapping_table[index] = readl_relaxed(bus->cnfg +
 						SPMI_MAPPING_TABLE_REG(index));
 
 		data = mapping_table[index];
@@ -1016,9 +1044,9 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
 			} else {
 				apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
-				pmic_arb->ppid_to_apid[ppid]
+				bus->ppid_to_apid[ppid]
 					= apid | PMIC_ARB_APID_VALID;
-				pmic_arb->apid_data[apid].ppid = ppid;
+				bus->apid_data[apid].ppid = ppid;
 				return apid;
 			}
 		} else {
@@ -1026,9 +1054,9 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
 			} else {
 				apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
-				pmic_arb->ppid_to_apid[ppid]
+				bus->ppid_to_apid[ppid]
 					= apid | PMIC_ARB_APID_VALID;
-				pmic_arb->apid_data[apid].ppid = ppid;
+				bus->apid_data[apid].ppid = ppid;
 				return apid;
 			}
 		}
@@ -1038,24 +1066,26 @@ static int pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 }
 
 /* v1 offset per ee */
-static int pmic_arb_offset_v1(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
-			enum pmic_arb_channel ch_type)
+static int pmic_arb_offset_v1(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
+			      enum pmic_arb_channel ch_type)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return 0x800 + 0x80 * pmic_arb->channel;
 }
 
-static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
+static u16 pmic_arb_find_apid(struct spmi_pmic_arb_bus *bus, u16 ppid)
 {
-	struct apid_data *apidd = &pmic_arb->apid_data[pmic_arb->last_apid];
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
+	struct apid_data *apidd = &bus->apid_data[bus->last_apid];
 	u32 regval, offset;
 	u16 id, apid;
 
-	for (apid = pmic_arb->last_apid; ; apid++, apidd++) {
+	for (apid = bus->last_apid; ; apid++, apidd++) {
 		offset = pmic_arb->ver_ops->apid_map_offset(apid);
 		if (offset >= pmic_arb->core_size)
 			break;
 
-		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
+		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(bus,
 								     apid));
 		apidd->irq_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
 		apidd->write_ee = apidd->irq_ee;
@@ -1065,14 +1095,14 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 			continue;
 
 		id = (regval >> 8) & PMIC_ARB_PPID_MASK;
-		pmic_arb->ppid_to_apid[id] = apid | PMIC_ARB_APID_VALID;
+		bus->ppid_to_apid[id] = apid | PMIC_ARB_APID_VALID;
 		apidd->ppid = id;
 		if (id == ppid) {
 			apid |= PMIC_ARB_APID_VALID;
 			break;
 		}
 	}
-	pmic_arb->last_apid = apid & ~PMIC_ARB_APID_VALID;
+	bus->last_apid = apid & ~PMIC_ARB_APID_VALID;
 
 	return apid;
 }
@@ -1104,21 +1134,22 @@ static int pmic_arb_get_core_resources_v2(struct platform_device *pdev,
 	return pmic_arb_get_obsrvr_chnls_v2(pdev);
 }
 
-static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pmic_arb, u16 ppid)
+static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb_bus *bus, u16 ppid)
 {
 	u16 apid_valid;
 
-	apid_valid = pmic_arb->ppid_to_apid[ppid];
+	apid_valid = bus->ppid_to_apid[ppid];
 	if (!(apid_valid & PMIC_ARB_APID_VALID))
-		apid_valid = pmic_arb_find_apid(pmic_arb, ppid);
+		apid_valid = pmic_arb_find_apid(bus, ppid);
 	if (!(apid_valid & PMIC_ARB_APID_VALID))
 		return -ENODEV;
 
 	return apid_valid & ~PMIC_ARB_APID_VALID;
 }
 
-static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
+static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb_bus *bus)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	struct apid_data *apidd;
 	struct apid_data *prev_apidd;
 	u16 i, apid, ppid, apid_max;
@@ -1140,9 +1171,9 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
 	 * where N = number of APIDs supported by the primary bus and
 	 *       M = number of APIDs supported by the secondary bus
 	 */
-	apidd = &pmic_arb->apid_data[pmic_arb->base_apid];
-	apid_max = pmic_arb->base_apid + pmic_arb->apid_count;
-	for (i = pmic_arb->base_apid; i < apid_max; i++, apidd++) {
+	apidd = &bus->apid_data[bus->base_apid];
+	apid_max = bus->base_apid + bus->apid_count;
+	for (i = bus->base_apid; i < apid_max; i++, apidd++) {
 		offset = pmic_arb->ver_ops->apid_map_offset(i);
 		if (offset >= pmic_arb->core_size)
 			break;
@@ -1153,19 +1184,18 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
 		ppid = (regval >> 8) & PMIC_ARB_PPID_MASK;
 		is_irq_ee = PMIC_ARB_CHAN_IS_IRQ_OWNER(regval);
 
-		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
-								     i));
+		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(bus, i));
 		apidd->write_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
 
 		apidd->irq_ee = is_irq_ee ? apidd->write_ee : INVALID_EE;
 
-		valid = pmic_arb->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID;
-		apid = pmic_arb->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
-		prev_apidd = &pmic_arb->apid_data[apid];
+		valid = bus->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID;
+		apid = bus->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
+		prev_apidd = &bus->apid_data[apid];
 
 		if (!valid || apidd->write_ee == pmic_arb->ee) {
 			/* First PPID mapping or one for this EE */
-			pmic_arb->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
+			bus->ppid_to_apid[ppid] = i | PMIC_ARB_APID_VALID;
 		} else if (valid && is_irq_ee &&
 			   prev_apidd->write_ee == pmic_arb->ee) {
 			/*
@@ -1176,42 +1206,43 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
 		}
 
 		apidd->ppid = ppid;
-		pmic_arb->last_apid = i;
+		bus->last_apid = i;
 	}
 
 	/* Dump the mapping table for debug purposes. */
-	dev_dbg(&pmic_arb->spmic->dev, "PPID APID Write-EE IRQ-EE\n");
+	dev_dbg(&bus->spmic->dev, "PPID APID Write-EE IRQ-EE\n");
 	for (ppid = 0; ppid < PMIC_ARB_MAX_PPID; ppid++) {
-		apid = pmic_arb->ppid_to_apid[ppid];
+		apid = bus->ppid_to_apid[ppid];
 		if (apid & PMIC_ARB_APID_VALID) {
 			apid &= ~PMIC_ARB_APID_VALID;
-			apidd = &pmic_arb->apid_data[apid];
-			dev_dbg(&pmic_arb->spmic->dev, "%#03X %3u %2u %2u\n",
-			      ppid, apid, apidd->write_ee, apidd->irq_ee);
+			apidd = &bus->apid_data[apid];
+			dev_dbg(&bus->spmic->dev, "%#03X %3u %2u %2u\n",
+				ppid, apid, apidd->write_ee, apidd->irq_ee);
 		}
 	}
 
 	return 0;
 }
 
-static int pmic_arb_ppid_to_apid_v5(struct spmi_pmic_arb *pmic_arb, u16 ppid)
+static int pmic_arb_ppid_to_apid_v5(struct spmi_pmic_arb_bus *bus, u16 ppid)
 {
-	if (!(pmic_arb->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID))
+	if (!(bus->ppid_to_apid[ppid] & PMIC_ARB_APID_VALID))
 		return -ENODEV;
 
-	return pmic_arb->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
+	return bus->ppid_to_apid[ppid] & ~PMIC_ARB_APID_VALID;
 }
 
 /* v2 offset per ppid and per ee */
-static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
-			   enum pmic_arb_channel ch_type)
+static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
+			      enum pmic_arb_channel ch_type)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u16 apid;
 	u16 ppid;
 	int rc;
 
 	ppid = sid << 8 | ((addr >> 8) & 0xFF);
-	rc = pmic_arb_ppid_to_apid_v2(pmic_arb, ppid);
+	rc = pmic_arb_ppid_to_apid_v2(bus, ppid);
 	if (rc < 0)
 		return rc;
 
@@ -1219,27 +1250,28 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
 	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
 }
 
-static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb)
+static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	int ret;
 
-	pmic_arb->base_apid = 0;
-	pmic_arb->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
-					   PMIC_ARB_FEATURES_PERIPH_MASK;
+	bus->base_apid = 0;
+	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+					PMIC_ARB_FEATURES_PERIPH_MASK;
 
-	if (pmic_arb->base_apid + pmic_arb->apid_count > pmic_arb->max_periphs) {
-		dev_err(&pmic_arb->spmic->dev, "Unsupported APID count %d detected\n",
-			pmic_arb->base_apid + pmic_arb->apid_count);
+	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
+		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
+			bus->base_apid + bus->apid_count);
 		return -EINVAL;
 	}
 
-	ret = pmic_arb_init_apid_min_max(pmic_arb);
+	ret = pmic_arb_init_apid_min_max(bus);
 	if (ret)
 		return ret;
 
-	ret = pmic_arb_read_apid_map_v5(pmic_arb);
+	ret = pmic_arb_read_apid_map_v5(bus);
 	if (ret) {
-		dev_err(&pmic_arb->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
+		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
 			ret);
 		return ret;
 	}
@@ -1251,15 +1283,16 @@ static int pmic_arb_init_apid_v5(struct spmi_pmic_arb *pmic_arb)
  * v5 offset per ee and per apid for observer channels and per apid for
  * read/write channels.
  */
-static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
-			   enum pmic_arb_channel ch_type)
+static int pmic_arb_offset_v5(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
+			      enum pmic_arb_channel ch_type)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u16 apid;
 	int rc;
 	u32 offset = 0;
 	u16 ppid = (sid << 8) | (addr >> 8);
 
-	rc = pmic_arb_ppid_to_apid_v5(pmic_arb, ppid);
+	rc = pmic_arb_ppid_to_apid_v5(bus, ppid);
 	if (rc < 0)
 		return rc;
 
@@ -1269,8 +1302,8 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
 		offset = 0x10000 * pmic_arb->ee + 0x80 * apid;
 		break;
 	case PMIC_ARB_CHANNEL_RW:
-		if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
-			dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
+		if (bus->apid_data[apid].write_ee != pmic_arb->ee) {
+			dev_err(&bus->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
 				sid, addr);
 			return -EPERM;
 		}
@@ -1297,15 +1330,16 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
  * v7 offset per ee and per apid for observer channels and per apid for
  * read/write channels.
  */
-static int pmic_arb_offset_v7(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
-			   enum pmic_arb_channel ch_type)
+static int pmic_arb_offset_v7(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
+			      enum pmic_arb_channel ch_type)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u16 apid;
 	int rc;
 	u32 offset = 0;
 	u16 ppid = (sid << 8) | (addr >> 8);
 
-	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
+	rc = pmic_arb->ver_ops->ppid_to_apid(bus, ppid);
 	if (rc < 0)
 		return rc;
 
@@ -1315,8 +1349,8 @@ static int pmic_arb_offset_v7(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
 		offset = 0x8000 * pmic_arb->ee + 0x20 * apid;
 		break;
 	case PMIC_ARB_CHANNEL_RW:
-		if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
-			dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
+		if (bus->apid_data[apid].write_ee != pmic_arb->ee) {
+			dev_err(&bus->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
 				sid, addr);
 			return -EPERM;
 		}
@@ -1338,104 +1372,110 @@ static u32 pmic_arb_fmt_cmd_v2(u8 opc, u8 sid, u16 addr, u8 bc)
 }
 
 static void __iomem *
-pmic_arb_owner_acc_status_v1(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+pmic_arb_owner_acc_status_v1(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
 {
-	return pmic_arb->intr + 0x20 * m + 0x4 * n;
+	return bus->intr + 0x20 * m + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_owner_acc_status_v2(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+pmic_arb_owner_acc_status_v2(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
 {
-	return pmic_arb->intr + 0x100000 + 0x1000 * m + 0x4 * n;
+	return bus->intr + 0x100000 + 0x1000 * m + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_owner_acc_status_v3(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+pmic_arb_owner_acc_status_v3(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
 {
-	return pmic_arb->intr + 0x200000 + 0x1000 * m + 0x4 * n;
+	return bus->intr + 0x200000 + 0x1000 * m + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_owner_acc_status_v5(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+pmic_arb_owner_acc_status_v5(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
 {
-	return pmic_arb->intr + 0x10000 * m + 0x4 * n;
+	return bus->intr + 0x10000 * m + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_owner_acc_status_v7(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+pmic_arb_owner_acc_status_v7(struct spmi_pmic_arb_bus *bus, u8 m, u16 n)
 {
-	return pmic_arb->intr + 0x1000 * m + 0x4 * n;
+	return bus->intr + 0x1000 * m + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_acc_enable_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_acc_enable_v1(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0x200 + 0x4 * n;
+	return bus->intr + 0x200 + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_acc_enable_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_acc_enable_v2(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0x1000 * n;
+	return bus->intr + 0x1000 * n;
 }
 
 static void __iomem *
-pmic_arb_acc_enable_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_acc_enable_v5(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x100 + 0x10000 * n;
 }
 
 static void __iomem *
-pmic_arb_acc_enable_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_acc_enable_v7(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x100 + 0x1000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_status_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_status_v1(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0x600 + 0x4 * n;
+	return bus->intr + 0x600 + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_status_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_status_v2(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0x4 + 0x1000 * n;
+	return bus->intr + 0x4 + 0x1000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_status_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_status_v5(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x104 + 0x10000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_status_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_status_v7(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x104 + 0x1000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_clear_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_clear_v1(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0xA00 + 0x4 * n;
+	return bus->intr + 0xA00 + 0x4 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_clear_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_clear_v2(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->intr + 0x8 + 0x1000 * n;
+	return bus->intr + 0x8 + 0x1000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_clear_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_clear_v5(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x108 + 0x10000 * n;
 }
 
 static void __iomem *
-pmic_arb_irq_clear_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_irq_clear_v7(struct spmi_pmic_arb_bus *bus, u16 n)
 {
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	return pmic_arb->wr_base + 0x108 + 0x1000 * n;
 }
 
@@ -1455,9 +1495,9 @@ static u32 pmic_arb_apid_map_offset_v7(u16 n)
 }
 
 static void __iomem *
-pmic_arb_apid_owner_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_apid_owner_v2(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->cnfg + 0x700 + 0x4 * n;
+	return bus->cnfg + 0x700 + 0x4 * n;
 }
 
 /*
@@ -1466,9 +1506,9 @@ pmic_arb_apid_owner_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
  * 0.
  */
 static void __iomem *
-pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+pmic_arb_apid_owner_v7(struct spmi_pmic_arb_bus *bus, u16 n)
 {
-	return pmic_arb->cnfg + 0x4 * (n - pmic_arb->base_apid);
+	return bus->cnfg + 0x4 * (n - bus->base_apid);
 }
 
 static const struct pmic_arb_ver_ops pmic_arb_v1 = {
@@ -1558,29 +1598,121 @@ static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
 	.translate = qpnpint_irq_domain_translate,
 };
 
+static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
+				  struct device_node *node,
+				  struct spmi_pmic_arb *pmic_arb)
+{
+	struct spmi_pmic_arb_bus *bus;
+	struct device *dev = &pdev->dev;
+	struct spmi_controller *ctrl;
+	void __iomem *intr;
+	void __iomem *cnfg;
+	int index, ret;
+	u32 irq;
+
+	ctrl = devm_spmi_controller_alloc(dev, sizeof(*bus));
+	if (IS_ERR(ctrl))
+		return PTR_ERR(ctrl);
+
+	ctrl->cmd = pmic_arb_cmd;
+	ctrl->read_cmd = pmic_arb_read_cmd;
+	ctrl->write_cmd = pmic_arb_write_cmd;
+
+	bus = spmi_controller_get_drvdata(ctrl);
+
+	pmic_arb->bus = bus;
+
+	raw_spin_lock_init(&bus->lock);
+
+	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
+					 sizeof(*bus->ppid_to_apid),
+					 GFP_KERNEL);
+	if (!bus->ppid_to_apid)
+		return -ENOMEM;
+
+	bus->apid_data = devm_kcalloc(dev, pmic_arb->max_periphs,
+				      sizeof(*bus->apid_data),
+				      GFP_KERNEL);
+	if (!bus->apid_data)
+		return -ENOMEM;
+
+	index = of_property_match_string(node, "reg-names", "cnfg");
+	if (index < 0) {
+		dev_err(dev, "cnfg reg region missing");
+		return -EINVAL;
+	}
+
+	cnfg = devm_of_iomap(dev, node, index, NULL);
+	if (IS_ERR(cnfg))
+		return PTR_ERR(cnfg);
+
+	index = of_property_match_string(node, "reg-names", "intr");
+	if (index < 0) {
+		dev_err(dev, "intr reg region missing");
+		return -EINVAL;
+	}
+
+	intr = devm_of_iomap(dev, node, index, NULL);
+	if (IS_ERR(intr))
+		return PTR_ERR(intr);
+
+	irq = of_irq_get_byname(node, "periph_irq");
+	if (irq < 0)
+		return irq;
+
+	bus->pmic_arb = pmic_arb;
+	bus->intr = intr;
+	bus->cnfg = cnfg;
+	bus->irq = irq;
+	bus->spmic = ctrl;
+
+	ret = pmic_arb->ver_ops->init_apid(bus);
+	if (ret)
+		return ret;
+
+	dev_dbg(&pdev->dev, "adding irq domain\n");
+
+	bus->domain = irq_domain_add_tree(dev->of_node,
+					  &pmic_arb_irq_domain_ops, bus);
+	if (!bus->domain) {
+		dev_err(&pdev->dev, "unable to create irq_domain\n");
+		return -ENOMEM;
+	}
+
+	irq_set_chained_handler_and_data(bus->irq,
+					 pmic_arb_chained_irq, bus);
+
+	ctrl->dev.of_node = node;
+
+	ret = devm_spmi_controller_add(dev, ctrl);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int spmi_pmic_arb_probe(struct platform_device *pdev)
 {
 	struct spmi_pmic_arb *pmic_arb;
-	struct spmi_controller *ctrl;
+	struct device *dev = &pdev->dev;
 	struct resource *res;
 	void __iomem *core;
 	u32 channel, ee, hw_ver;
 	int err;
 
-	ctrl = devm_spmi_controller_alloc(&pdev->dev, sizeof(*pmic_arb));
-	if (IS_ERR(ctrl))
-		return PTR_ERR(ctrl);
-
-	pmic_arb = spmi_controller_get_drvdata(ctrl);
-	pmic_arb->spmic = ctrl;
+	pmic_arb = devm_kzalloc(dev, sizeof(*pmic_arb), GFP_KERNEL);
+	if (!pmic_arb)
+		return -ENOMEM;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
-	core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
+	core = devm_ioremap(dev, res->start, resource_size(res));
 	if (IS_ERR(core))
 		return PTR_ERR(core);
 
 	pmic_arb->core_size = resource_size(res);
 
+	platform_set_drvdata(pdev, pmic_arb);
+
 	hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
 
 	if (hw_ver < PMIC_ARB_VERSION_V2_MIN)
@@ -1594,30 +1726,12 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 	else
 		pmic_arb->ver_ops = &pmic_arb_v7;
 
-	dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
-		 pmic_arb->ver_ops->ver_str, hw_ver);
-
 	err = pmic_arb->ver_ops->get_core_resources(pdev, core);
 	if (err)
 		return err;
 
-	err = pmic_arb->ver_ops->init_apid(pmic_arb);
-	if (err)
-		return err;
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
-	pmic_arb->intr = devm_ioremap_resource(&ctrl->dev, res);
-	if (IS_ERR(pmic_arb->intr))
-		return PTR_ERR(pmic_arb->intr);
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cnfg");
-	pmic_arb->cnfg = devm_ioremap_resource(&ctrl->dev, res);
-	if (IS_ERR(pmic_arb->cnfg))
-		return PTR_ERR(pmic_arb->cnfg);
-
-	pmic_arb->irq = platform_get_irq_byname(pdev, "periph_irq");
-	if (pmic_arb->irq < 0)
-		return pmic_arb->irq;
+	dev_info(dev, "PMIC arbiter version %s (0x%x)\n",
+		 pmic_arb->ver_ops->ver_str, hw_ver);
 
 	err = of_property_read_u32(pdev->dev.of_node, "qcom,channel", &channel);
 	if (err) {
@@ -1646,42 +1760,17 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 
 	pmic_arb->ee = ee;
 
-	platform_set_drvdata(pdev, ctrl);
-	raw_spin_lock_init(&pmic_arb->lock);
-
-	ctrl->cmd = pmic_arb_cmd;
-	ctrl->read_cmd = pmic_arb_read_cmd;
-	ctrl->write_cmd = pmic_arb_write_cmd;
-
-	dev_dbg(&pdev->dev, "adding irq domain\n");
-	pmic_arb->domain = irq_domain_add_tree(pdev->dev.of_node,
-					 &pmic_arb_irq_domain_ops, pmic_arb);
-	if (!pmic_arb->domain) {
-		dev_err(&pdev->dev, "unable to create irq_domain\n");
-		return -ENOMEM;
-	}
-
-	irq_set_chained_handler_and_data(pmic_arb->irq, pmic_arb_chained_irq,
-					pmic_arb);
-	err = spmi_controller_add(ctrl);
-	if (err)
-		goto err_domain_remove;
-
-	return 0;
-
-err_domain_remove:
-	irq_set_chained_handler_and_data(pmic_arb->irq, NULL, NULL);
-	irq_domain_remove(pmic_arb->domain);
-	return err;
+	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
 }
 
 static void spmi_pmic_arb_remove(struct platform_device *pdev)
 {
-	struct spmi_controller *ctrl = platform_get_drvdata(pdev);
-	struct spmi_pmic_arb *pmic_arb = spmi_controller_get_drvdata(ctrl);
-	spmi_controller_remove(ctrl);
-	irq_set_chained_handler_and_data(pmic_arb->irq, NULL, NULL);
-	irq_domain_remove(pmic_arb->domain);
+	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
+	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
+
+	irq_set_chained_handler_and_data(bus->irq,
+					 NULL, NULL);
+	irq_domain_remove(bus->domain);
 }
 
 static const struct of_device_id spmi_pmic_arb_match_table[] = {

-- 
2.34.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 v9 7/7] spmi: pmic-arb: Add multi bus support
From: Abel Vesa @ 2024-04-07 16:23 UTC (permalink / raw)
  To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Srini Kandagatla, Johan Hovold, David Collins, linux-kernel,
	linux-arm-kernel, linux-arm-msm, linux-mediatek, devicetree,
	Abel Vesa
In-Reply-To: <20240407-spmi-multi-master-support-v9-0-fa151c1391f3@linaro.org>

Starting with HW version 7, there are actually two separate buses
(with two separate sets of wires). So add support for the second bus.
The first platform that needs this support for the second bus is the
Qualcomm X1 Elite, so add the compatible for it as well.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 120 insertions(+), 18 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 65d8f45da24b..cfa8ec21cd56 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -95,6 +96,8 @@ enum pmic_arb_channel {
 	PMIC_ARB_CHANNEL_OBS,
 };
 
+#define PMIC_ARB_MAX_BUSES		2
+
 /* Maximum number of support PMIC peripherals */
 #define PMIC_ARB_MAX_PERIPHS		512
 #define PMIC_ARB_MAX_PERIPHS_V7		1024
@@ -149,6 +152,7 @@ struct spmi_pmic_arb;
  * @min_apid:		minimum APID (used for bounding IRQ search)
  * @max_apid:		maximum APID
  * @irq:		PMIC ARB interrupt.
+ * @id:			unique ID of the bus
  */
 struct spmi_pmic_arb_bus {
 	struct spmi_pmic_arb	*pmic_arb;
@@ -167,6 +171,7 @@ struct spmi_pmic_arb_bus {
 	u16			min_apid;
 	u16			max_apid;
 	int			irq;
+	u8			id;
 };
 
 /**
@@ -180,7 +185,8 @@ struct spmi_pmic_arb_bus {
  * @ee:			the current Execution Environment
  * @ver_ops:		version dependent operations.
  * @max_periphs:	Number of elements in apid_data[]
- * @bus:		per arbiter bus instance
+ * @buses:		per arbiter buses instances
+ * @buses_available:	number of buses registered
  */
 struct spmi_pmic_arb {
 	void __iomem		*rd_base;
@@ -191,7 +197,8 @@ struct spmi_pmic_arb {
 	u8			ee;
 	const struct pmic_arb_ver_ops *ver_ops;
 	int			max_periphs;
-	struct spmi_pmic_arb_bus *bus;
+	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
+	int			buses_available;
 };
 
 /**
@@ -220,7 +227,7 @@ struct spmi_pmic_arb {
 struct pmic_arb_ver_ops {
 	const char *ver_str;
 	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
-	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
+	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
 	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
 	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
 	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
@@ -309,8 +316,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 			}
 
 			if (status & PMIC_ARB_STATUS_FAILURE) {
-				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
-					__func__, sid, addr, status);
+				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
+					__func__, sid, addr, status, offset);
 				WARN_ON(1);
 				return -EIO;
 			}
@@ -326,8 +333,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 		udelay(1);
 	}
 
-	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
-		__func__, sid, addr, status);
+	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
+		__func__, bus->id, sid, addr, status);
 	return -ETIMEDOUT;
 }
 
@@ -1003,11 +1010,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
 	return 0;
 }
 
-static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
+static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
 {
 	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u32 *mapping_table;
 
+	if (index) {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			index);
+		return -EINVAL;
+	}
+
 	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
 				     sizeof(*mapping_table), GFP_KERNEL);
 	if (!mapping_table)
@@ -1250,11 +1263,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
 	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
 }
 
-static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
+static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
 {
 	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	int ret;
 
+	if (index) {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			index);
+		return -EINVAL;
+	}
+
 	bus->base_apid = 0;
 	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
 					PMIC_ARB_FEATURES_PERIPH_MASK;
@@ -1326,6 +1345,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
 	return pmic_arb_get_obsrvr_chnls_v2(pdev);
 }
 
+/*
+ * Only v7 supports 2 buses. Each bus will get a different apid count, read
+ * from different registers.
+ */
+static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
+{
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
+	int ret;
+
+	if (index == 0) {
+		bus->base_apid = 0;
+		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+						   PMIC_ARB_FEATURES_PERIPH_MASK;
+	} else if (index == 1) {
+		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+						  PMIC_ARB_FEATURES_PERIPH_MASK;
+		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
+						   PMIC_ARB_FEATURES_PERIPH_MASK;
+	} else {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			bus->id);
+		return -EINVAL;
+	}
+
+	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
+		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
+			bus->base_apid + bus->apid_count);
+		return -EINVAL;
+	}
+
+	ret = pmic_arb_init_apid_min_max(bus);
+	if (ret)
+		return ret;
+
+	ret = pmic_arb_read_apid_map_v5(bus);
+	if (ret) {
+		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * v7 offset per ee and per apid for observer channels and per apid for
  * read/write channels.
@@ -1578,7 +1641,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 static const struct pmic_arb_ver_ops pmic_arb_v7 = {
 	.ver_str		= "v7",
 	.get_core_resources	= pmic_arb_get_core_resources_v7,
-	.init_apid		= pmic_arb_init_apid_v5,
+	.init_apid		= pmic_arb_init_apid_v7,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
 	.offset			= pmic_arb_offset_v7,
@@ -1602,6 +1665,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 				  struct device_node *node,
 				  struct spmi_pmic_arb *pmic_arb)
 {
+	int bus_index = pmic_arb->buses_available;
 	struct spmi_pmic_arb_bus *bus;
 	struct device *dev = &pdev->dev;
 	struct spmi_controller *ctrl;
@@ -1620,7 +1684,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 
 	bus = spmi_controller_get_drvdata(ctrl);
 
-	pmic_arb->bus = bus;
+	pmic_arb->buses[bus_index] = bus;
 
 	raw_spin_lock_init(&bus->lock);
 
@@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 	bus->cnfg = cnfg;
 	bus->irq = irq;
 	bus->spmic = ctrl;
+	bus->id = bus_index;
 
-	ret = pmic_arb->ver_ops->init_apid(bus);
+	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
 	if (ret)
 		return ret;
 
-	dev_dbg(&pdev->dev, "adding irq domain\n");
+	dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
 
 	bus->domain = irq_domain_add_tree(dev->of_node,
 					  &pmic_arb_irq_domain_ops, bus);
@@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 					 pmic_arb_chained_irq, bus);
 
 	ctrl->dev.of_node = node;
+	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
 
 	ret = devm_spmi_controller_add(dev, ctrl);
 	if (ret)
 		return ret;
 
+	pmic_arb->buses_available++;
+
 	return 0;
 }
 
+static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
+					struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *child;
+	int ret;
+
+	/* legacy mode doesn't provide child node for the bus */
+	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
+		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
+
+	for_each_available_child_of_node(node, child) {
+		if (of_node_name_eq(child, "spmi")) {
+			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return ret;
+}
+
+static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
+{
+	int i;
+
+	for (i = 0; i < pmic_arb->buses_available; i++) {
+		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
+
+		irq_set_chained_handler_and_data(bus->irq,
+						 NULL, NULL);
+		irq_domain_remove(bus->domain);
+	}
+}
+
 static int spmi_pmic_arb_probe(struct platform_device *pdev)
 {
 	struct spmi_pmic_arb *pmic_arb;
@@ -1760,21 +1864,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 
 	pmic_arb->ee = ee;
 
-	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
+	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
 }
 
 static void spmi_pmic_arb_remove(struct platform_device *pdev)
 {
 	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
-	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
 
-	irq_set_chained_handler_and_data(bus->irq,
-					 NULL, NULL);
-	irq_domain_remove(bus->domain);
+	spmi_pmic_arb_deregister_buses(pmic_arb);
 }
 
 static const struct of_device_id spmi_pmic_arb_match_table[] = {
 	{ .compatible = "qcom,spmi-pmic-arb", },
+	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);

-- 
2.34.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 v9 5/7] spmi: pmic-arb: Make core resources acquiring a version operation
From: Abel Vesa @ 2024-04-07 16:23 UTC (permalink / raw)
  To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
	Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Srini Kandagatla, Johan Hovold, David Collins, linux-kernel,
	linux-arm-kernel, linux-arm-msm, linux-mediatek, devicetree,
	Abel Vesa
In-Reply-To: <20240407-spmi-multi-master-support-v9-0-fa151c1391f3@linaro.org>

Rather than setting up the core, obsrv and chnls in probe by using
version specific conditionals, add a dedicated "get_core_resources"
version specific op and move the acquiring in there. Since there are
no current users of the second bus yet, drop the comment about why
devm_platform_ioremap_resource can't be used in case of "core",
as it is not applicable anymore.
Don't switch to devm_platform_ioremap_resource though as we need
to keep track of core size.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/spmi/spmi-pmic-arb.c | 114 +++++++++++++++++++++++++++----------------
 1 file changed, 71 insertions(+), 43 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index dc969f8bed18..ff777b4a6f33 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -186,6 +186,7 @@ struct spmi_pmic_arb {
  * struct pmic_arb_ver_ops - version dependent functionality.
  *
  * @ver_str:		version string.
+ * @get_core_resources:	initializes the core, observer and channels
  * @init_apid:		finds the apid base and count
  * @ppid_to_apid:	finds the apid for a given ppid.
  * @non_data_cmd:	on v1 issues an spmi non-data command.
@@ -206,6 +207,7 @@ struct spmi_pmic_arb {
  */
 struct pmic_arb_ver_ops {
 	const char *ver_str;
+	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
 	int (*init_apid)(struct spmi_pmic_arb *pmic_arb);
 	int (*ppid_to_apid)(struct spmi_pmic_arb *pmic_arb, u16 ppid);
 	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
@@ -961,6 +963,19 @@ static int pmic_arb_init_apid_min_max(struct spmi_pmic_arb *pmic_arb)
 	return 0;
 }
 
+static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
+					  void __iomem *core)
+{
+	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
+
+	pmic_arb->wr_base = core;
+	pmic_arb->rd_base = core;
+
+	pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
+
+	return 0;
+}
+
 static int pmic_arb_init_apid_v1(struct spmi_pmic_arb *pmic_arb)
 {
 	u32 *mapping_table;
@@ -1062,6 +1077,33 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 	return apid;
 }
 
+static int pmic_arb_get_obsrvr_chnls_v2(struct platform_device *pdev)
+{
+	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
+
+	pmic_arb->rd_base = devm_platform_ioremap_resource_byname(pdev, "obsrvr");
+	if (IS_ERR(pmic_arb->rd_base))
+		return PTR_ERR(pmic_arb->rd_base);
+
+	pmic_arb->wr_base = devm_platform_ioremap_resource_byname(pdev, "chnls");
+	if (IS_ERR(pmic_arb->wr_base))
+		return PTR_ERR(pmic_arb->wr_base);
+
+	return 0;
+}
+
+static int pmic_arb_get_core_resources_v2(struct platform_device *pdev,
+					  void __iomem *core)
+{
+	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
+
+	pmic_arb->core = core;
+
+	pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
+
+	return pmic_arb_get_obsrvr_chnls_v2(pdev);
+}
+
 static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 {
 	u16 apid_valid;
@@ -1239,6 +1281,18 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
 	return offset;
 }
 
+static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
+					  void __iomem *core)
+{
+	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
+
+	pmic_arb->core = core;
+
+	pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
+
+	return pmic_arb_get_obsrvr_chnls_v2(pdev);
+}
+
 /*
  * v7 offset per ee and per apid for observer channels and per apid for
  * read/write channels.
@@ -1419,6 +1473,7 @@ pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
 
 static const struct pmic_arb_ver_ops pmic_arb_v1 = {
 	.ver_str		= "v1",
+	.get_core_resources	= pmic_arb_get_core_resources_v1,
 	.init_apid		= pmic_arb_init_apid_v1,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v1,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v1,
@@ -1434,6 +1489,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v1 = {
 
 static const struct pmic_arb_ver_ops pmic_arb_v2 = {
 	.ver_str		= "v2",
+	.get_core_resources	= pmic_arb_get_core_resources_v2,
 	.init_apid		= pmic_arb_init_apid_v1,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v2,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
@@ -1449,6 +1505,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v2 = {
 
 static const struct pmic_arb_ver_ops pmic_arb_v3 = {
 	.ver_str		= "v3",
+	.get_core_resources	= pmic_arb_get_core_resources_v2,
 	.init_apid		= pmic_arb_init_apid_v1,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v2,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
@@ -1464,6 +1521,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v3 = {
 
 static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 	.ver_str		= "v5",
+	.get_core_resources	= pmic_arb_get_core_resources_v2,
 	.init_apid		= pmic_arb_init_apid_v5,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
@@ -1479,6 +1537,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 
 static const struct pmic_arb_ver_ops pmic_arb_v7 = {
 	.ver_str		= "v7",
+	.get_core_resources	= pmic_arb_get_core_resources_v7,
 	.init_apid		= pmic_arb_init_apid_v5,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
@@ -1515,16 +1574,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 	pmic_arb = spmi_controller_get_drvdata(ctrl);
 	pmic_arb->spmic = ctrl;
 
-	/*
-	 * Please don't replace this with devm_platform_ioremap_resource() or
-	 * devm_ioremap_resource().  These both result in a call to
-	 * devm_request_mem_region() which prevents multiple mappings of this
-	 * register address range.  SoCs with PMIC arbiter v7 may define two
-	 * arbiter devices, for the two physical SPMI interfaces, which  share
-	 * some register address ranges (i.e. "core", "obsrvr", and "chnls").
-	 * Ensure that both devices probe successfully by calling devm_ioremap()
-	 * which does not result in a devm_request_mem_region() call.
-	 */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
 	core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
 	if (IS_ERR(core))
@@ -1534,44 +1583,23 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 
 	hw_ver = readl_relaxed(core + PMIC_ARB_VERSION);
 
-	if (hw_ver < PMIC_ARB_VERSION_V2_MIN) {
+	if (hw_ver < PMIC_ARB_VERSION_V2_MIN)
 		pmic_arb->ver_ops = &pmic_arb_v1;
-		pmic_arb->wr_base = core;
-		pmic_arb->rd_base = core;
-	} else {
-		pmic_arb->core = core;
-
-		if (hw_ver < PMIC_ARB_VERSION_V3_MIN)
-			pmic_arb->ver_ops = &pmic_arb_v2;
-		else if (hw_ver < PMIC_ARB_VERSION_V5_MIN)
-			pmic_arb->ver_ops = &pmic_arb_v3;
-		else if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
-			pmic_arb->ver_ops = &pmic_arb_v5;
-		else
-			pmic_arb->ver_ops = &pmic_arb_v7;
-
-		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   "obsrvr");
-		pmic_arb->rd_base = devm_ioremap(&ctrl->dev, res->start,
-						 resource_size(res));
-		if (IS_ERR(pmic_arb->rd_base))
-			return PTR_ERR(pmic_arb->rd_base);
-
-		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   "chnls");
-		pmic_arb->wr_base = devm_ioremap(&ctrl->dev, res->start,
-						 resource_size(res));
-		if (IS_ERR(pmic_arb->wr_base))
-			return PTR_ERR(pmic_arb->wr_base);
-	}
+	else if (hw_ver < PMIC_ARB_VERSION_V3_MIN)
+		pmic_arb->ver_ops = &pmic_arb_v2;
+	else if (hw_ver < PMIC_ARB_VERSION_V5_MIN)
+		pmic_arb->ver_ops = &pmic_arb_v3;
+	else if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
+		pmic_arb->ver_ops = &pmic_arb_v5;
+	else
+		pmic_arb->ver_ops = &pmic_arb_v7;
 
 	dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
 		 pmic_arb->ver_ops->ver_str, hw_ver);
 
-	if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
-		pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
-	else
-		pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
+	err = pmic_arb->ver_ops->get_core_resources(pdev, core);
+	if (err)
+		return err;
 
 	err = pmic_arb->ver_ops->init_apid(pmic_arb);
 	if (err)

-- 
2.34.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