Hi, Krzysztof,
On 10/22/24 5:38 AM, Krzysztof Kozlowski wrote:
cut
>>> I skimmed through the driver and I do not understand why this is
>>> firmware. You are implementing a mailbox provider/controller.
>>
>> In my case the mailbox hardware is used just to raise the interrupt to
>> the other side. Then there's the SRAM which contains the channels
>> configuration data and the TX/RX queues. The enqueue/deque is done
>> in/from SRAM. This resembles a lot with drivers/firmware/arm_scmi/, see:
>>
>> drivers/firmware/arm_scmi/shmem.c
>> drivers/firmware/arm_scmi/transports/mailbox.c
>
> Wait, SCMI is an interface. Not the case here.
>
>>
>> After the SRAM and mailbox/transport code I'll come up with two helper
>> drivers that construct the mailbox messages in the format expected by
>> the firmware. There are 2 types of messages recognized by the ACPM
>> firmware: PMIC and DVFS. The client drivers will use these helper
>> drivers to prepare a specific message. Then they will use the mailbox
>> core to send the message and they'll wait for the answer.
>>
>> This layered structure and the use of SRAM resembles with arm_scmi and
>> made me think that the ACPM driver it's better suited for
>> drivers/firmware. I'm opened for suggestions though.
>
> Sure, but then this driver cannot perform mbox_controller_register().
> Only mailbox providers, so drivers in mailbox, use it.
>
Okay, I can move the driver to drivers/mailbox/.
cut
>>>> +/**
>>>> + * struct exynos_acpm_shmem_chan - descriptor of a shared memory channel.
>>>> + *
>>>> + * @id: channel ID.
>>>> + * @reserved: reserved for future use.
>>>> + * @rx_rear: rear pointer of RX queue.
>>>> + * @rx_front: front pointer of RX queue.
>>>> + * @rx_base: base address of RX queue.
>>>> + * @reserved1: reserved for future use.
>>>> + * @tx_rear: rear pointer of TX queue.
>>>> + * @tx_front: front pointer of TX queue.
>>>> + * @tx_base: base address of TX queue.
>>>> + * @qlen: queue length. Applies to both TX/RX queues.
>>>> + * @mlen: message length. Applies to both TX/RX queues.
>>>> + * @reserved2: reserved for future use.
>>>> + * @polling: true when the channel works on polling.
>>>> + */
>>>> +struct exynos_acpm_shmem_chan {
>>>> + u32 id;
>>>> + u32 reserved[3];
>>>> + u32 rx_rear;
>>>> + u32 rx_front;
>>>> + u32 rx_base;
>>>> + u32 reserved1[3];
>>>> + u32 tx_rear;
>>>> + u32 tx_front;
>>>> + u32 tx_base;
>>>> + u32 qlen;
>>>> + u32 mlen;
>>>> + u32 reserved2[2];
>>>> + u32 polling;
>>>
cut
>>>
>>> I also cannot find any piece of code setting several of above, e.g. tx_base
>>
>> I'm not writing any SRAM configuration fields, these fields are used to
>> read/retrive the channel parameters from SRAM.
>
> I meany tx_base is always 0. Where is this property set? Ever?
It's not zero. My assumption is it is set in the acpm firmware, but I
don't have access to that to verify. Here are some debug prints made in
the linux driver:
[ 0.069575][ T1] gs-acpm-ipc 17610000.mailbox:
exynos_mbox_chan_init ID = 2 poll = 1, mlen = 16, qlen = 5
[ 0.069927][ T1] gs-acpm-ipc 17610000.mailbox:
exynos_mbox_chan_init ID = 2 offsets: rx_base = 0x00038290 rx_front =
0x0003828c, rx_rear = 0x00038288
[ 0.070449][ T1] gs-acpm-ipc 17610000.mailbox:
exynos_mbox_chan_init ID = 2 offsets: tx_base = 0x000382f0 tx_front =
0x000382ec, tx_rear = 0x000382e8
tx_base contains the SRAM offset of the RX queue used in linux. The
offset is relative to the base address of the SRAM config data.
tx_base is seen/named from the firmware's point of view, thus named TX.
I assume the same struct is defined in the acpm firmware.
Somewhere below in the linux driver I get the RX ring base address by doing:
rx->base = exynos_acpm_get_iomem_addr(base, &shmem_chan->tx_base);
where base is the SRAM base address of the channels configuration data.
static void __iomem *exynos_acpm_get_iomem_addr(void __iomem *base,
void __iomem *addr)
{
u32 offset;
offset = readl_relaxed(addr);
return base + offset;
}
Hope this clarifies a bit these struct members.
Cheers,
ta
