[PATCH V3 16/21] i3c: mipi-i3c-hci: Factor out core initialization into helper
Frank Li
Frank.li at nxp.com
Tue Jan 13 09:48:48 PST 2026
On Tue, Jan 13, 2026 at 07:27:05PM +0200, Adrian Hunter wrote:
> On 13/01/2026 18:27, Frank Li wrote:
> > On Tue, Jan 13, 2026 at 09:26:57AM +0200, Adrian Hunter wrote:
> >> Prepare for future reuse. Move core initialization logic from
> >> i3c_hci_init() into a dedicated helper function,
> >> i3c_hci_reset_and_init().
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
> >> ---
> >>
> >>
> >> Changes in V3:
> >>
> >> Move I/O mode setting if I/O mode already selected, to a
> >> separate patch
> >>
> >> Changes in V2:
> >>
> >> New patch
> >>
> >>
> >> drivers/i3c/master/mipi-i3c-hci/core.c | 142 +++++++++++++------------
> >> 1 file changed, 75 insertions(+), 67 deletions(-)
> >>
> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> >> index 569c8584045a..8b8e3952d41d 100644
> >> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> >> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> >> @@ -639,6 +639,80 @@ static int i3c_hci_set_io_mode(struct i3c_hci *hci, bool dma)
> >> return 0;
> >> }
> >>
> >> +static int i3c_hci_reset_and_init(struct i3c_hci *hci)
> >> +{
> >> + u32 regval;
> >> + int ret;
> >> +
> >> + ret = i3c_hci_software_reset(hci);
> >> + if (ret)
> >> + return -ENXIO;
> >> +
> >> + /* Disable all interrupts */
> >> + reg_write(INTR_SIGNAL_ENABLE, 0x0);
> >> + /*
> >> + * Only allow bit 31:10 signal updates because
> >> + * Bit 0:9 are reserved in IP version >= 0.8
> >> + * Bit 0:5 are defined in IP version < 0.8 but not handled by PIO code
> >> + */
> >> + reg_write(INTR_STATUS_ENABLE, GENMASK(31, 10));
> >> +
> >> + /* Make sure our data ordering fits the host's */
> >> + regval = reg_read(HC_CONTROL);
> >> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> >> + if (!(regval & HC_CONTROL_DATA_BIG_ENDIAN)) {
> >> + regval |= HC_CONTROL_DATA_BIG_ENDIAN;
> >> + reg_write(HC_CONTROL, regval);
> >> + regval = reg_read(HC_CONTROL);
> >> + if (!(regval & HC_CONTROL_DATA_BIG_ENDIAN)) {
> >> + dev_err(&hci->master.dev, "cannot set BE mode\n");
> >> + return -EOPNOTSUPP;
> >> + }
> >> + }
> >> + } else {
> >> + if (regval & HC_CONTROL_DATA_BIG_ENDIAN) {
> >> + regval &= ~HC_CONTROL_DATA_BIG_ENDIAN;
> >> + reg_write(HC_CONTROL, regval);
> >> + regval = reg_read(HC_CONTROL);
> >> + if (regval & HC_CONTROL_DATA_BIG_ENDIAN) {
> >> + dev_err(&hci->master.dev, "cannot clear BE mode\n");
> >> + return -EOPNOTSUPP;
> >> + }
> >> + }
> >> + }
> >> +
> >> + /* Try activating DMA operations first */
> >> + if (hci->RHS_regs) {
> >> + ret = i3c_hci_set_io_mode(hci, true);
> >> + if (!ret) {
> >> + hci->io = &mipi_i3c_hci_dma;
> >> + dev_dbg(&hci->master.dev, "Using DMA\n");
> >> + }
> >> + }
> >> +
> >> + /* If no DMA, try PIO */
> >> + if (!hci->io && hci->PIO_regs) {
> >> + ret = i3c_hci_set_io_mode(hci, false);
> >> + if (!ret) {
> >> + hci->io = &mipi_i3c_hci_pio;
> >> + dev_dbg(&hci->master.dev, "Using PIO\n");
> >> + }
> >> + }
> >> +
> >> + if (!hci->io) {
> >> + dev_err(&hci->master.dev, "neither DMA nor PIO can be used\n");
> >> + ret = ret ?: -EINVAL;
> >> + }
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Configure OD and PP timings for AMD platforms */
> >> + if (hci->quirks & HCI_QUIRK_OD_PP_TIMING)
> >> + amd_set_od_pp_timing(hci);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static int i3c_hci_init(struct i3c_hci *hci)
> >> {
> >> bool size_in_dwords;
> >> @@ -708,43 +782,6 @@ static int i3c_hci_init(struct i3c_hci *hci)
> >> if (ret)
> >> return ret;
> >>
> >> - ret = i3c_hci_software_reset(hci);
> >> - if (ret)
> >> - return -ENXIO;
> >> -
> >> - /* Disable all interrupts */
> >> - reg_write(INTR_SIGNAL_ENABLE, 0x0);
> >> - /*
> >> - * Only allow bit 31:10 signal updates because
> >> - * Bit 0:9 are reserved in IP version >= 0.8
> >> - * Bit 0:5 are defined in IP version < 0.8 but not handled by PIO code
> >> - */
> >> - reg_write(INTR_STATUS_ENABLE, GENMASK(31, 10));
> >> -
> >> - /* Make sure our data ordering fits the host's */
> >> - regval = reg_read(HC_CONTROL);
> >> - if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) {
> >> - if (!(regval & HC_CONTROL_DATA_BIG_ENDIAN)) {
> >> - regval |= HC_CONTROL_DATA_BIG_ENDIAN;
> >> - reg_write(HC_CONTROL, regval);
> >> - regval = reg_read(HC_CONTROL);
> >> - if (!(regval & HC_CONTROL_DATA_BIG_ENDIAN)) {
> >> - dev_err(&hci->master.dev, "cannot set BE mode\n");
> >> - return -EOPNOTSUPP;
> >> - }
> >> - }
> >> - } else {
> >> - if (regval & HC_CONTROL_DATA_BIG_ENDIAN) {
> >> - regval &= ~HC_CONTROL_DATA_BIG_ENDIAN;
> >> - reg_write(HC_CONTROL, regval);
> >> - regval = reg_read(HC_CONTROL);
> >> - if (regval & HC_CONTROL_DATA_BIG_ENDIAN) {
> >> - dev_err(&hci->master.dev, "cannot clear BE mode\n");
> >> - return -EOPNOTSUPP;
> >> - }
> >> - }
> >> - }
> >> -
> >> /* Select our command descriptor model */
> >> switch (FIELD_GET(HC_CAP_CMD_SIZE, hci->caps)) {
> >> case 0:
> >> @@ -762,36 +799,7 @@ static int i3c_hci_init(struct i3c_hci *hci)
> >> if (hci->quirks & HCI_QUIRK_PIO_MODE)
> >> hci->RHS_regs = NULL;
> >
> > these parts have not move into helper funciton. Is it safe to use
> > "hci->RHS_regs" in helper funciton?
>
> These parts are initializing things that never need to be
> re-initialized.
>
> RHS_regs is the MMIO address of the Ring Headers section
> for DMA, if there is one. It is normally setup only once and
> never changed, but the quirk above sets it to zero to prevent
> DMA from being used.
>
> The I/O mode (DMA or PIO) is selected during probe and never
> changed.
Okay
Reviewed-by: Frank Li <Frank.Li at nxp.com>
>
> >
> > Frank
> >
> >>
> >> - /* Try activating DMA operations first */
> >> - if (hci->RHS_regs) {
> >> - ret = i3c_hci_set_io_mode(hci, true);
> >> - if (!ret) {
> >> - hci->io = &mipi_i3c_hci_dma;
> >> - dev_dbg(&hci->master.dev, "Using DMA\n");
> >> - }
> >> - }
> >> -
> >> - /* If no DMA, try PIO */
> >> - if (!hci->io && hci->PIO_regs) {
> >> - ret = i3c_hci_set_io_mode(hci, false);
> >> - if (!ret) {
> >> - hci->io = &mipi_i3c_hci_pio;
> >> - dev_dbg(&hci->master.dev, "Using PIO\n");
> >> - }
> >> - }
> >> -
> >> - if (!hci->io) {
> >> - dev_err(&hci->master.dev, "neither DMA nor PIO can be used\n");
> >> - if (!ret)
> >> - ret = -EINVAL;
> >> - return ret;
> >> - }
> >> -
> >> - /* Configure OD and PP timings for AMD platforms */
> >> - if (hci->quirks & HCI_QUIRK_OD_PP_TIMING)
> >> - amd_set_od_pp_timing(hci);
> >> -
> >> - return 0;
> >> + return i3c_hci_reset_and_init(hci);
> >> }
> >>
> >> static int i3c_hci_probe(struct platform_device *pdev)
> >> --
> >> 2.51.0
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list