Bus cleanup path in DMA mode may trigger a RING_OP_STAT interrupt when
the ring is being stopped. Depending on timing between ring stop request
completion, interrupt handler removal and code execution this may lead
to a NULL pointer dereference in hci_dma_irq_handler() if it gets to run
after the io_data pointer is set to NULL in hci_dma_cleanup().
Prevent this my masking the ring interrupts before ring stop request.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240920144432.62370-2-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
svc_i3c_master_do_daa() {
...
for (i = 0; i < dev_nb; i++) {
ret = i3c_master_add_i3c_dev_locked(m, addrs[i]);
if (ret)
goto rpm_out;
}
}
If two devices (A and B) are detected in DAA and address 0xa is assigned to
device A and 0xb to device B, a failure in i3c_master_add_i3c_dev_locked()
for device A (addr: 0xa) could prevent device B (addr: 0xb) from being
registered on the bus. The I3C stack might still consider 0xb a free
address. If a subsequent Hotjoin occurs, 0xb might be assigned to Device A,
causing both devices A and B to use the same address 0xb, violating the I3C
specification.
The return value for i3c_master_add_i3c_dev_locked() should not be checked
because subsequent steps will scan the entire I3C bus, independent of
whether i3c_master_add_i3c_dev_locked() returns success.
If device A registration fails, there is still a chance to register device
B. i3c_master_add_i3c_dev_locked() can reset DAA if a failure occurs while
retrieving device information.
Cc: stable@kernel.org
Fixes: 317bacf960 ("i3c: master: add enable(disable) hot join in sys entry")
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-6-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Wait for the controller to complete emitting ACK/NACK, otherwise the next
command may be omitted by the hardware.
Add a "command done" check in svc_i3c_master_nack(ack)_ibi() and change the
return type to int to flag possible timeouts.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-5-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
According to I3C spec ver 1.1, 09-Jun-2021, section 5.1.2.5:
The I3C Controller shall hold SCL low while the Bus is in ACK/NACK Phase of
I3C/I2C transfer. But maximum stall time is 100us. The IRQs have to be
disabled to prevent schedule during the whole I3C transaction, otherwise,
the I3C bus timeout may happen if any irq or schedule happen during
transaction.
Replace mutex with spin_lock_irqsave() to avoid stalling SCL more than
100us.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-4-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
When sending REQUEST_PROC_DAA, emit START and address 7E. Address
arbitration may occur at this time if other devices trigger HJ, IBI, or
CR events.
When IBIWON happen during a REQUEST_PROC_DAA, NACK the IBI request then
send a repeated start to continue current dynamic address assignment.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-3-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
When the address is arbitrated at send address, the hardware can auto-send
NACK if it is an IBI. However, manual emission of NACK/ACK is needed for
hot join or controller request events.
Add help function svc_i3c_master_handle_ibi_won() to check event type and
send out NACK if the event is not an IBI.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-2-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
There is a possibility of an IBI WIN occurring when addressing issues, even
when sending CCC commands. Most of the time, returning -EAGAIN is
acceptable, but the case below becomes highly complex.
When a Hotjoin event occurs:
- i3c_master_do_daa()
- i3c_master_add_i3c_dev_locked()
- A dynamic address (e.g., 0x9) is already set during DAA.
- i3c_master_getpid_locked()
- Another device issues HJ or IBI here. Returning -EAGAIN causes
failure in adding the new device. However, the dynamic address(0x9)
has already been assigned to this device. If another device issues
HJ, it will get this address 0x9 again, causing two devices on the
bus to use the same dynamic address 0x9.
- Attempting to send RSTDAA when the first device fails at
i3c_master_getpid_locked() could also fail when sending RSTDAA for
the same reason.
According to the I3C spec, address arbitration only happens at START, never
at REPEAT start. Using repeat start when an IBI WIN occurs simplifies this
case, as i3c_master_getpid_locked() will not return an error when another
device tries to send HJ or IBI.
Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20241002-svc-i3c-hj-v6-1-7e6e1d3569ae@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In the svc_i3c_master_probe function, &master->hj_work is bound with
svc_i3c_master_hj_work, &master->ibi_work is bound with
svc_i3c_master_ibi_work. And svc_i3c_master_ibi_work can start the
hj_work, svc_i3c_master_irq_handler can start the ibi_work.
If we remove the module which will call svc_i3c_master_remove to
make cleanup, it will free master->base through i3c_master_unregister
while the work mentioned above will be used. The sequence of operations
that may lead to a UAF bug is as follows:
CPU0 CPU1
| svc_i3c_master_hj_work
svc_i3c_master_remove |
i3c_master_unregister(&master->base)|
device_unregister(&master->dev) |
device_release |
//free master->base |
| i3c_master_do_daa(&master->base)
| //use master->base
Fix it by ensuring that the work is canceled before proceeding with the
cleanup in svc_i3c_master_remove.
Fixes: 0f74f8b667 ("i3c: Make i3c_master_unregister() return void")
Cc: stable@vger.kernel.org
Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/stable/20240914154030.180-1-kxwang23%40m.fudan.edu.cn
Link: https://lore.kernel.org/r/20240914163932.253-1-kxwang23@m.fudan.edu.cn
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In the cdns_i3c_master_probe function, &master->hj_work is bound with
cdns_i3c_master_hj. And cdns_i3c_master_interrupt can call
cnds_i3c_master_demux_ibis function to start the work.
If we remove the module which will call cdns_i3c_master_remove to
make cleanup, it will free master->base through i3c_master_unregister
while the work mentioned above will be used. The sequence of operations
that may lead to a UAF bug is as follows:
CPU0 CPU1
| cdns_i3c_master_hj
cdns_i3c_master_remove |
i3c_master_unregister(&master->base) |
device_unregister(&master->dev) |
device_release |
//free master->base |
| i3c_master_do_daa(&master->base)
| //use master->base
Fix it by ensuring that the work is canceled before proceeding with
the cleanup in cdns_i3c_master_remove.
Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn>
Link: https://lore.kernel.org/r/20240911153544.848398-1-kxwang23@m.fudan.edu.cn
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
According to I3C Specification(Version 1.1) 5.1.2.4 "Use of Clock
Speed to Prevent Legacy I2C Devices From Seeing I3C traffic", when
slow i2c devices(FM/FM+ rate i2c frequency without 50ns filter)
works on i3c bus, i3c SDR should work at FM/FM+ rate.
Adjust timing for difference mode.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Frank Li <frank.li@nxp.com>
Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20240719080233.842771-1-carlos.song@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
I3C controller should support adjusting open drain timing for the first
broadcast address to make I3C device working as a i2c device can see slow
broadcast address to close its Spike Filter to change working at i3c mode.
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Reviewed-by: Frank Li <frank.li@nxp.com>
Link: https://lore.kernel.org/r/20240910051626.4052552-2-carlos.song@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Based on the I3C TCRI specification, the rules for determining the I3C
mode are as follows:
I3C SCL rate > 8MHz: use SDR0, as SDR1 has a maximum data rate of 8MHz
I3C SCL rate > 6MHz: use SDR1, as SDR2 has a maximum data rate of 6MHz
I3C SCL rate > 4MHz: use SDR2, as SDR3 has a maximum data rate of 4MHz
I3C SCL rate > 2MHz: use SDR3, as SDR4 has a maximum data rate of 2MHz
Otherwise, use SDR4
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Link: https://lore.kernel.org/r/20240826033821.175591-1-billy_tsai@aspeedtech.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add MODULE_DEVICE_TABLE(), so modules could be properly autoloaded
based on the alias from of_device_id table.
Signed-off-by: Liao Chen <liaochen4@huawei.com>
Link: https://lore.kernel.org/r/20240826123957.379212-1-liaochen4@huawei.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The current driver sets the response buffer threshold value to 1
(N+1, 2 DWORDS) in the QUEUE THRESHOLD register. However, the AMD
I3C controller only generates interrupts when the response buffer
threshold value is set to 0 (1 DWORD).
Therefore, a quirk is added to set the response buffer threshold value
to 0.
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Co-developed-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Signed-off-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Link: https://lore.kernel.org/r/20240829091713.736217-7-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The AMD HCI controller is currently unstable at 12.5 MHz. To address this,
a quirk is added to configure the clock rate to 9 MHz as a workaround,
with proportional adjustments to the Open-Drain (OD) and Push-Pull (PP)
values.
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Link: https://lore.kernel.org/r/20240829091713.736217-6-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The reg_* helper macros are currently limited to core.c. Moving them to
hci.h will allow their functionality to be utilized in other files outside
of core.c.
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Link: https://lore.kernel.org/r/20240829091713.736217-5-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The AMD HCI controller currently only supports PIO mode but exposes DMA
rings to the OS, which leads to the controller being configured in DMA
mode. To address this, add a quirk to avoid configuring the controller in
DMA mode and default to PIO mode.
Additionally, introduce a generic quirk infrastructure to the mipi-i3c-hci
driver to facilitate seamless future quirk additions.
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Co-developed-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Signed-off-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Link: https://lore.kernel.org/r/20240829091713.736217-4-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The HC_CONTROL_PIO_MODE bit was introduced in the HC_CONTROL register
starting from version 1.1. Therefore, checking the HC_CONTROL_PIO_MODE bit
on hardware that adheres to older specification revisions (i.e., versions
earlier than 1.1) is incorrect. To address this, add an additional check
to read the HCI version before attempting to read the HC_CONTROL_PIO_MODE
status.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240829091713.736217-3-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The current driver code lacks the necessary plumbing for ACPI IDs,
preventing the mipi-i3c-hci driver from being loaded on x86
platforms that advertise I3C ACPI support.
Add the AMDI5017 ACPI ID to the list of supported IDs.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240829091713.736217-2-Shyam-sundar.S-k@amd.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
This code has a typo so it returns positive EIO instead of negative -EIO. Fix
it!
Fixes: a7809cb368b9 ("i3c: master: svc: Improve DAA STOP handle code logic")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/e017edfc-da64-496b-8516-958bec27cd9a@stanley.mountain
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add support for runtime and system power management.
Handle clocks, resets, pads as part of suspend and resume.
Restore controller registers that could be lost due to suspend.
Finally add get and put calls appropriately in functions which
access controller : bus_init, do_daa, send_ccc_cmd, priv_xfers,
i2c_xfers, ibi and hot-join.
Signed-off-by: Aniket <aniketmaurya@google.com>
Link: https://lore.kernel.org/r/20240708062103.3296587-4-aniketmaurya@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Separate logic for setting intr/thld registers in a func.
Also modify enable function to take care of setting all fields in DEVICE_CTRL.
These functions can be reused later for power management.
Signed-off-by: Aniket <aniketmaurya@google.com>
Link: https://lore.kernel.org/r/20240708062103.3296587-3-aniketmaurya@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add variables to store timing registers and other values.
These variables would be later used to restore registers
during resume without recomputation.
Signed-off-by: Aniket <aniketmaurya@google.com>
Link: https://lore.kernel.org/r/20240708062103.3296587-2-aniketmaurya@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The REQUEST_PROC_DAA command behaves differently from other commands.
Sometimes the hardware can auto emit STOP, but in other conditions, it
cannot.
Improves the code logic to better handle these situations.
Hardware can auto emit STOP only when the following conditions are met:
- The previous I3C device correctly returns a PID and ACKs an I3C address.
- A NACK is received when emitting 7E to try to get the next I3C device's
PID.
In all other cases, a manual STOP emission is needed.
The code is changed to emit STOP when break the while loop and 'return 0'
only when the hardware can auto emit STOP.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20240702223107.403057-1-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Move to "enabled" variant of clk_get API. It takes care
of enable and disable calls during the probe and remove.
Signed-off-by: Aniket <aniketmaurya@google.com>
Link: https://lore.kernel.org/r/20240628154603.326075-1-aniketmaurya@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The dma.c: hci_dma_init() sets the CHUNK_SIZE field in the IBI_SETUP
register incorrectly if the calculated ibi_chunk_sz is not exactly
2^(n+2) bytes, where n is 0..6.
Fix this by rounding the chunk size up to nearest 2^(n+2) bytes.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240628131559.502822-4-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Definitely condition dma_get_cache_alignment * defined value > 256
during driver initialization is not reason to BUG_ON(). Turn that to
graceful error out with -EINVAL.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240628131559.502822-3-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
IBI Status and Data Ring base address registers are not set so HW
obviously cannot update those rings after In-Band Interrupt.
Set them to already allocated and mapped ring addresses.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240628131559.502822-2-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Rather than having own lo32()/hi32() helpers for dealing with 32-bit and
64-bit build targets switch to generic lower_32_bits()/upper_32_bits()
helpers.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20240628131559.502822-1-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Since DW I3C IP master role always supports IBI, we don't need
to keep two variants of master ops and select one using this
property. Hence remove the code.
Signed-off-by: Aniket <aniketmaurya@google.com>
Link: https://lore.kernel.org/r/20240627034119.3938050-1-aniketmaurya@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
IBI_SIR_REQ_REJECT register is not present if the IP has
IC_HAS_IBI_DATA = 1 set. So don't rely on doing read-
modify-write op on this register.
Instead maintain a variable to store the sir reject mask
and use it to set IBI_SIR_REQ_REJECT.
Signed-off-by: Aniket <aniketmaurya@google.com>
Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
I was wrong about the TABLE_SIZE field description in the
commit 0676bfebf5 ("i3c: mipi-i3c-hci: Fix DAT/DCT entry sizes").
For the MIPI I3C HCI versions 1.0 and earlier the TABLE_SIZE field in
the registers DAT_SECTION_OFFSET and DCT_SECTION_OFFSET is indeed defined
in DWORDs and not number of entries like it is defined in later versions.
Where above fix allowed driver initialization to continue the wrongly
interpreted TABLE_SIZE field leads variables DAT_entries being twice and
DCT_entries four times as big as they really are.
That in turn leads clearing the DAT table over the boundary in the
dat_v1.c: hci_dat_v1_init().
So interprete the TABLE_SIZE field in DWORDs for HCI versions < 1.1 and
fix number of DAT/DCT entries accordingly.
Fixes: 0676bfebf5 ("i3c: mipi-i3c-hci: Fix DAT/DCT entry sizes")
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
According to I3C Spec 1.1.1, 11-Jun-2021, section: 5.1.2.2.3:
If the Controller chooses to start an I3C Message with an I3C Dynamic
Address, then special provisions shall be made because that same I3C Target
may be initiating an IBI or a Controller Role Request. So, one of three
things may happen: (skip 1, 2)
3. The Addresses match and the RnW bits also match, and so neither
Controller nor Target will ACK since both are expecting the other side to
provide ACK. As a result, each side might think it had "won" arbitration,
but neither side would continue, as each would subsequently see that the
other did not provide ACK.
...
For either value of RnW: Due to the NACK, the Controller shall defer the
Private Write or Private Read, and should typically transmit the Target
^^^^^^^^^^^^^^^^^^^
Address again after a Repeated START (i.e., the next one or any one prior
^^^^^^^^^^^^^
to a STOP in the Frame). Since the Address Header following a Repeated
START is not arbitrated, the Controller will always win (see Section
5.1.2.2.4).
Resend target address again if address is not 7E and controller get NACK.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add hot-join support for dw i3c master controller.
By default, the hot-join acknowledgment is disabled, and the hardware will
automatically send the DISEC CCC when it receives the hot-join request.
Users can use the sys entry to enable it.
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Link: https://lore.kernel.org/r/20240429073624.256830-1-billy_tsai@aspeedtech.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In an In-Band Interrupt (IBI) handle, the code logic is as follows:
1: writel(SVC_I3C_MCTRL_REQUEST_AUTO_IBI | SVC_I3C_MCTRL_IBIRESP_AUTO,
master->regs + SVC_I3C_MCTRL);
2: ret = readl_relaxed_poll_timeout(master->regs + SVC_I3C_MSTATUS, val,
SVC_I3C_MSTATUS_IBIWON(val), 0, 1000);
...
3: ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
SVC_I3C_MSTATUS_IBIWON may be set before step 1. Thus, step 2 will return
immediately, and the I3C controller has not sent out the 9th SCL yet.
Consequently, ibitype and ibiaddr are 0, resulting in an unknown IBI type
occurrence and missing call I3C client driver's IBI handler.
A typical case is that SVC_I3C_MSTATUS_IBIWON is set when an IBI occurs
during the controller send start frame in svc_i3c_master_xfer().
Clear SVC_I3C_MSTATUS_IBIWON before issue SVC_I3C_MCTRL_REQUEST_AUTO_IBI
to fix this issue.
Cc: stable@vger.kernel.org
Fixes: 5e5e3c92e7 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20240506164009.21375-3-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
svc_i3c_master_xfer() returns error ENXIO if an In-Band Interrupt (IBI)
occurs when the host starts the frame.
Change error code to EAGAIN to inform the client driver that this
situation has occurred and to try again sometime later.
Fixes: 5e5e3c92e7 ("i3c: master: svc: fix wrong data return when IBI happen during start frame")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20240506164009.21375-2-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Disable IBI IRQ signal and status only when hot-join and SIR enabling of
all target devices attached to the bus are disabled.
Fixes: e389b1d72a ("i3c: dw: Add support for in-band interrupts")
Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Link: https://lore.kernel.org/r/20240119054547.983693-1-dylan_hung@aspeedtech.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
As per the Cadence IP document fixed the I2C clock divider value limit from
16 bits instead of 10 bits. Without this change setting up the I2C clock to
low frequencies will not work as the prescaler value might be greater than
10 bit number.
I3C clock divider value is 10 bits only. Updating the macro names for both.
Signed-off-by: Harshit Shah <harshitshah.opendev@gmail.com>
Link: https://lore.kernel.org/r/1703927483-28682-1-git-send-email-harshitshah.opendev@gmail.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
I3C allow devices early terminate data transfer. So set "actual_len" to
indicate how much data get by i3c_priv_xfer.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231201222532.2431484-6-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
I3C transfer (SDR), target can early terminate read transfer.
I3C transfer (HDR), target can end write transfer.
I2C transfer, target can NACK write transfer.
'actual_len' is better name than 'read_len'.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231201222532.2431484-5-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add hot join support for svc master controller. Disable hot join by
default.
User can use sysfs entry to enable hot join.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231201222532.2431484-3-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Implement a local bounce buffer for private I3C SDR and I2C transfers
when using DMA and the buffer attached to the transfer is not DMA safe.
Otherwise the DMA transfer will fail and with following warning:
[ 11.411059] i3c mipi-i3c-hci.0: rejecting DMA map of vmalloc memory
[ 11.417313] WARNING: CPU: 3 PID: 357 at include/linux/dma-mapping.h:332 hci_dma_queue_xfer+0x2e2/0x300 [mipi_i3c_hci]
Strictly speaking private I3C SDR transfers are expected to pass a
DMA-able buffer. However I fear this requirement may easily be slipped
or go unnoticed when I3C interface support is added into a existing
device driver that use regmap API to read/write stack variables.
For example this is the case with the commit 2660b0080b ("iio: imu:
st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR").
Buffer of an I2C message is not required to be DMA safe and the I2C core
provides i2c_(get|put)_dma_safe_msg_buf() helpers for the host
controllers that do DMA and that is also recommendation for the
i2c_xfers() callback from the I3C core.
However due to above I3C private transfers reason I decided to implement
a bounce buffer for them and reuse the same code for the I2C transfers
too. Since this driver is currently the only I3C host controller driver
that can do DMA the implementation is done here and not in I3C core.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20231109133708.653950-5-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Handle also I3C address header error response status as the end of DAA
process in hci_cmd_v1_daa().
According to MIPI I3C HCI Specification v1.1 the NACK error during DAA
process comes when the device does not accept the dynamic address.
Currently code uses it for successful exit from the process and fails
with any other error response.
I'm unsure is this MIPI I3C HCI version specific difference or
specification misunderstanding but on an early MIPI I3C HCI version
compatible controller responds always with I3C address header error and
not with NACK error when there is no device on the bus or no more devices
participating to DAA process.
Handle now both response statuses as the end of DAA.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20231109133708.653950-4-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Function hci_cmd_v1_daa() uses only single transfer at a time so no need
to allocate two transfers and access can be simplified.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20231109133708.653950-3-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Currently probe of mipi-i3c-hci will fail if bus doesn't have any I3C
devices connected. This happens when CCC commands that are sent during
i3c_master_bus_init() are not ACKed by any device and controller responds
with an error status set.
The controller can detect NACK both during I3C address header
transmission (broadcast address 0x7e is not ACKed) and when target
device address or dynamic address assignment is NACKed. Former as error
status 0x4: Address Header Error and latter as 0x5: NACK.
Difference between those two NACK statuses were not described explicitly
until MIPI I3C HCI Specification v1.1. Earlier versions share the same
error status code though.
Report both of those as I3C_ERROR_M2 to I3C core code.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20231109133708.653950-2-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The `i3c_master_bus_init` function may attach the I2C devices before the
I3C bus initialization. In this flow, the DAT `alloc_entry`` will be used
before the DAT `init`. Additionally, if the `i3c_master_bus_init` fails,
the DAT `cleanup` will execute before the device is detached, which will
execue DAT `free_entry` function. The above scenario can cause the driver
to use DAT_data when it is NULL.
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Link: https://lore.kernel.org/r/20231023080237.560936-1-billy_tsai@aspeedtech.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In the binding documentation, the compatible string is specified as
'silvaco,i3c-master-v1', but in the driver, it is defined as
'silvaco,i3c-master'.
Rename 'silvaco,i3c-master' to 'silvaco,i3c-master-v1' to ensure
compatibility with the documentation.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231017194657.3199749-1-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
master side report:
silvaco-i3c-master 44330000.i3c-master: Error condition: MSTATUS 0x020090c7, MERRWARN 0x00100000
BIT 20: TIMEOUT error
The module has stalled too long in a frame. This happens when:
- The TX FIFO or RX FIFO is not handled and the bus is stuck in the
middle of a message,
- No STOP was issued and between messages,
- IBI manual is used and no decision was made.
The maximum stall period is 100 μs.
This can be considered as being just a warning as the system IRQ latency
can easily be greater than 100us.
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20231023161658.3890811-7-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Upon IBIWON timeout, the SDA line will always be kept low if we don't emit
a stop. Calling svc_i3c_master_emit_stop() there will let the bus return to
idle state.
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: <stable@vger.kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231023161658.3890811-6-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
MSTATUS[RXPEND] is only updated after the data transfer cycle started. This
creates an issue when the I3C clock is slow, and the CPU is running fast
enough that MSTATUS[RXPEND] may not be updated when the code reaches
checking point. As a result, mandatory data can be missed.
Add a wait for MSTATUS[COMPLETE] to ensure that all mandatory data is
already in FIFO. It also works without mandatory data.
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: <stable@vger.kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231023161658.3890811-4-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
┌─────┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┌─────
SCL: ┘ └─────┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┘
───┐ ┌─────┐ ┌─────┐ ┌───────────┐
SDA: └───────────────────────┘ └─────┘ └─────┘ └─────
xxx╱ ╲╱ ╲╱ ╲╱ ╲╱ ╲
: xxx╲IBI ╱╲ Addr(0x0a) ╱╲ RW ╱╲NACK╱╲ S ╱
If an In-Band Interrupt (IBI) occurs and IBI work thread is not immediately
scheduled, when svc_i3c_master_priv_xfers() initiates the I3C transfer and
attempts to send address 0x7e, the target interprets it as an
IBI handler and returns the target address 0x0a.
However, svc_i3c_master_priv_xfers() does not handle this case and proceeds
with other transfers, resulting in incorrect data being returned.
Add IBIWON check in svc_i3c_master_xfer(). In case this situation occurs,
return a failure to the driver.
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: <stable@vger.kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231023161658.3890811-3-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The ibi work thread operates asynchronously with other transfers, such as
svc_i3c_master_priv_xfers(). Introduce mutex protection to ensure the
completion of the entire i3c/i2c transaction.
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: <stable@vger.kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20231023161658.3890811-2-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The MIPI I3C spec refers to a Provisioned ID, since it is (sometimes)
provisioned at device manufacturing.
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20231003075339.197099-1-matt@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Host Controller goes to halt state after aborted transfer and needs to
be resumed by SW. Add this resuming to DMA mode code too.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-13-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
On an HW I'm using in enabling work the RESUME bit is not set in the
HC_CONTROLLER register when Host Controller goes to halt state. Value 1
should mean controller is suspended when reading and writing 1 resumes it.
Because of this erratic behaviour plain HC_CONTROL read and write back
won't resume the controller. Therefore do it by setting the RESUME bit
explicitly.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-12-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Due to missing completion object for the ENTDAA command transfer in the
hci_cmd_v1_daa() the wait_for_completion_timeout() will obviously
timeout even the transfer itself may succeed.
Fix this by setting the xfer->completion to the already initialized
completion object.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-11-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Fix following warning (with CONFIG_DMA_API_DEBUG) which happens with a
transfer without a data buffer.
DMA-API: i3c mipi-i3c-hci.0: device driver tries to free DMA memory it has not allocated [device address=0x0000000000000000] [size=0 bytes]
For those transfers the hci_dma_queue_xfer() doesn't create a mapping and
the DMA address pointer xfer->data_dma is not set.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-10-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Number of software enabled Ring Bundles must be set before using them.
Otherwise Ring will not start and may be power-gated by the Host
Controller.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-9-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
If there is a transfer error during i3c_master_bus_init() and code goes
doing the bus cleanup in i3c_hci_bus_cleanup() there is possibility that
i3c_hci_irq_handler() is running in parallel with hci->io->cleanup()
which can be racy.
Prevent this by waiting there is no pending interrupt on other CPU
before doing the IO cleanup.
This was observed with ring headers where first transfer failed and
sometimes transfer error or ring transfer abort interrupt was coming
simultaneously with the bus cleanup path.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-8-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Set ring start request together with ring enable in hci_dma_init(). This
causes the ring abort request in hci_dma_dequeue_xfer() will raise the
INTR_RING_OP (RING_OP_STAT in MIPI I3C HCI specification) interrupt in
the RH_INTR_STATUS register.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-7-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Ring Abort request will timeout in case there is an error in the Host
Controller interrupt delivery or Ring Header configuration. Using BUG()
makes hard to debug those cases.
Make it less severe and turn BUG() to WARN_ON().
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-6-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Do not loop over ring headers in hci_dma_irq_handler() that are not
allocated and enabled in hci_dma_init(). Otherwise out of bounds access
will occur from rings->headers[i] access when i >= number of allocated
ring headers.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-5-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
MIPI I3C HCI specification v1.1 describes the ENTRY_SIZE field for the
Device Address Table (DAT) and the Device Characteristics Table (DCT)
section offset registers (DAT_SECTION_OFFSET and DCT_SECTION_OFFSET).
That field is not documented in earlier version.
ENTRY_SIZE value 0 is meant to be backward compatible. For the DAT entry
size it is interpreted as 2 DWORDs (8-bytes) and for the DCT entry size
as 4 DWORDs (16-bytes). Values 1-15 are reserved for future use.
New version I believe fixes also the TABLE_SIZE field description.
Before it was defined in DWORDs which I believe is incorrect since the
DAT/DCT table entry structures, and sizes, are described having
8-bytes/16-bytes entries.
This is more clear in the specification v1.1 which states the TABLE_SIZE
fields are interpreted as number of entries in the DAT/DCT tables. I
believe this same holds also in earlier version, at least it makes more
sense.
Fix code accordingly and let the DAT_entry_size and the DCT_entry_size
variables carry the size as bytes. Which is how it is already
interpreted in the dat_v1.c: hci_dat_v1_init().
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-4-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add MODULE_ALIAS() in order to be able to autoload this driver when the
device is added as a platform device from another glue code driver.
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Link: https://lore.kernel.org/r/20230921055704.1087277-3-jarkko.nikula@linux.intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct svc_i3c_xfer.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Conor Culhane <conor.culhane@silvaco.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-i3c@lists.infradead.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230922175023.work.239-kees@kernel.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct hci_rings_data.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Nicolas Pitre <npitre@baylibre.com>
Cc: Len Baker <len.baker@gmx.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: linux-i3c@lists.infradead.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20230922175019.work.129-kees@kernel.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct cdns_i3c_xfer.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: "Przemysław Gaj" <pgaj@cadence.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-i3c@lists.infradead.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20230922175014.work.637-kees@kernel.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct dw_i3c_xfer.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: linux-i3c@lists.infradead.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/20230922175011.work.800-kees@kernel.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
I3C masters are expected to support hot-join. This means at initialization
time we might not yet discover any device and this should not be treated
as a fatal error.
During the DAA procedure which happens at probe time, if no device has
joined, all CCC will be NACKed (from a bus perspective). This leads to an
early return with an error code which fails the probe of the master.
Let's avoid this by just telling the core through an I3C_ERROR_M2
return command code that no device was discovered, which is a valid
situation. This way the master will no longer bail out and fail to probe
for a wrong reason.
Cc: stable@vger.kernel.org
Fixes: dd3c52846d ("i3c: master: svc: Add Silvaco I3C master driver")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230831141324.2841525-1-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The 'saved_regs' member of the 'svc_i3c_master' structure is not
described in the kernel doc, which produces the following warning:
Function parameter or member 'saved_regs' not described in 'svc_i3c_master'
Add the missing line in the kernel documentation of the parent
structure.
Fixes: 1c5ee2a77b ("i3c: master: svc: fix i3c suspend/resume issue")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308171435.0xQ82lvu-lkp@intel.com/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230817101853.16805-1-miquel.raynal@bootlin.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
It is not possible for platform_get_irq() to return 0. Use the
return value from platform_get_irq().
Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230803085149.149248-1-ruanjinjie@huawei.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The exit criteria for the DAA should check if the data length is equal to
1, instead of checking if the response status is equal to 1.
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Link: https://lore.kernel.org/r/20230802100909.2568215-1-billy_tsai@aspeedtech.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The DT of_device.h and of_platform.h date back to the separate
of_platform_bus_type before it as merged into the regular platform bus.
As part of that merge prepping Arm DT support 13 years ago, they
"temporarily" include each other. They also include platform_device.h
and of.h. As a result, there's a pretty much random mix of those include
files used throughout the tree. In order to detangle these headers and
replace the implicit includes with struct declarations, users need to
explicitly include the correct includes.
Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/20230714174623.4057784-1-robh@kernel.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Add NACK check after start byte is sent.
It is possible to detect early that a device is not on the bus
and avoid invalid transmissions thereafter.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230517033030.3068085-3-xiaoning.wang@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
pm_runtime_resume_and_get() may call sleep(). It cannot be used in
svc_i3c_master_start_xfer_locked(), because it is in a spin lock.
Move the pm runtime operations to svc_i3c_master_enqueue_xfer().
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Fixes: 05be23ef78 ("i3c: master: svc: add runtime pm support")
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230517033030.3068085-2-xiaoning.wang@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
When system suspends, if i3c module is powered down, register
value will lose.
Need to save the key registers before system suspend.
So save these registers value in runtime pm suspend, and restore them
if these register's value is different with the saved values
when runtime pm resume.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230517033030.3068085-1-xiaoning.wang@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
smatch reports
drivers/i3c/master/ast2600-i3c-master.c:121:34: warning: symbol
'ast2600_i3c_ops' was not declared. Should it be static?
This variable is only used in its defining file, so it should be static.
Signed-off-by: Tom Rix <trix@redhat.com>
Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/20230429134601.2688558-1-trix@redhat.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The 2k register setting is zero, OR-ing it in doesn't parallel the 2k
and 750 ohm pullups. We need a separate value for the 545 ohm setting.
Reported-by: Lukwinski Zbigniew <zbigniew.lukwinski@linux.intel.com>
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/20230428001849.1775559-1-jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The ast2600 i3c hardware is capable of IBIs, but we need a workaround
for a hardware issue with the I3C state machine handling IBI payloads
of specific lengths when PEC is not enabled. To avoid this, we need to
unconditionally enable PECs, at the consquence of losing a byte of data
when the device does not send a PEC.
Enable IBIs on the ast2600 platform, including an implementation of the
PEC workaround, which prints a warning when triggered.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/ba923b96d6d129024c975e8a0472c5b2fcb3af32.1680161823.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
On the AST2600 i3c controller, we'll need to apply a workaround for a
hardware issue with IBI payloads.
Introduce a platform hook to allow dw i3c platform implementations to
modify the DAT entry in IBI enable/disable to allow this workaround in a
future change.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/d5d76a8d2336d2a71886537f42e71d51db184df6.1680161823.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
This change adds support for receiving and dequeueing i3c IBIs.
By setting struct dw_i3c_master->ibi_capable before probe, a platform
implementation can select the IBI-enabled version of the i3c_master_ops,
enabling the global IBI infrastrcture for that controller.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/79daeefd7ccb7c935d0c159149df21a6c9a73ffa.1680161823.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In an upcoming change, we will want to store additional data about the
devices we have in the data address table.
Change the type of the DAT entries into a struct, which currently just
has the address data.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/9dc0d9e2857e851a0cf04819df48e5d31921f83e.1680161823.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In a future change we'll want to read from the IBI FIFO too, so turn
dw_i3c_read_rx_fifo() into a generic read with the FIFO register as a
parameter.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/827204789583dd86addffb47ecaeab9d67cf95d5.1680161823.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Now that we have platform-specific infrastructure for the dw i3c driver,
add platform support for the ASPEED AST2600 SoC.
The AST2600 has a small set of "i3c global" registers, providing
platform-level i3c configuration outside of the i3c core.
For the ast2600, we need a couple of extra setup operations:
- on probe: find the i3c global register set and parse the SDA pullup
resistor values
- on init: set the pullups accordingly, and set the i3c instance IDs
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/20230331091501.3800299-4-jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The dw i3c core can be integrated into various SoC devices. Platforms
that use this core may need a little configuration that is specific to
that platform.
Add some infrastructure to allow platform-specific behaviour: common
probe/remove functions, a set of platform hook operations, and a pointer
for platform-specific data in struct dw_i3c_master. Move the common api
into a new (i3c local) header file.
Platforms will provide their own struct platform_driver, which allocates
struct dw_i3c_master, does any platform-specific probe behaviour, and
calls into the common probe.
A future change will add new platform support that uses this
infrastructure.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/20230331091501.3800299-2-jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
In the clock setup path, we set the hardware DEV_CTRL_I2C_SLAVE_PRESENT
bit on a shared mode bus, then read-back this bit for the conditional
tCAS set.
Instead, just use the bus->mode setting for the conditional test.
While we're at it, add a little comment about why the conditional is
there.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/92a933566f7846708a00ad7f5a16ee8e6ed32d0e.1680156630.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
We currently assume that the rx_len of a read command will be as
submitted, but we may have a shorter read than expected.
This change populates the output i3c xfer length from the actually-read
length.
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://lore.kernel.org/r/f4fff7ab18dee1f662dc7a5a4111fcd921e6792b.1680156630.git.jk@codeconstruct.com.au
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230318233311.265186-6-u.kleine-koenig@pengutronix.de
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230318233311.265186-5-u.kleine-koenig@pengutronix.de
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230318233311.265186-4-u.kleine-koenig@pengutronix.de
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20230318233311.265186-3-u.kleine-koenig@pengutronix.de
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The function returned zero unconditionally. Switch the return type to void
and simplify the callers accordingly.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/r/20230318233311.265186-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
The driver can match only via the DT table so the table should be always
used and the of_match_ptr does not have any sense (this also allows ACPI
matching via PRP0001, even though it might not be relevant here). This
also fixes !CONFIG_OF error:
drivers/i3c/master/dw-i3c-master.c:1201:34: error: ‘dw_i3c_master_of_match’ defined but not used [-Werror=unused-const-variable=]
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20230312132535.352246-1-krzysztof.kozlowski@linaro.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Bus-speed could be default(12.5MHz) or defined by users in dts.
Dw-i3c-master should not hard-code the initial speed to be
I3C_BUS_TYP_I3C_SCL_RATE (12.5MHz)
And because of Synopsys's I3C controller limit (hcnt/lcnt register
length) and core-clk provided, there is a limit to bus speed, too.
For example, when core-clk is 250 MHz, the bus speed cannot be
lowered below 1MHz.
Tested: tested with an i3c sensor and captured with a logic analyzer.
Signed-off-by: Jack Chen <zenghuchen@google.com>
Link: https://lore.kernel.org/r/20230216151057.293764-1-zenghuchen@google.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
