[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1317bce4-60d4-b502-0792-ad57b56f2280@ieee.org>
Date: Wed, 13 Oct 2021 17:30:39 -0500
From: Alex Elder <elder@...e.org>
To: Sireesh Kodali <sireeshkodali1@...il.com>,
phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, elder@...nel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Subject: Re: [RFC PATCH 14/17] net: ipa: Add support for IPA v2
microcontroller
On 9/19/21 10:08 PM, Sireesh Kodali wrote:
> There are some minor differences between IPA v2.x and later revisions
> with regards to the uc. The biggeset difference is the shared memory's
> layout. There are also some changes to the command numbers, but these
> are not too important, since the mainline driver doesn't use them.
It's a shame that so much has to be rearranged when the
structure definitions are changed. If I spent more time
thinking about this I might suggest a different way of
abstracting the two, but for now this looks fine.
-Alex
> Signed-off-by: Sireesh Kodali <sireeshkodali1@...il.com>
> ---
> drivers/net/ipa/ipa_uc.c | 96 ++++++++++++++++++++++++++--------------
> 1 file changed, 63 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/ipa/ipa_uc.c b/drivers/net/ipa/ipa_uc.c
> index 856e55a080a7..bf6b25098301 100644
> --- a/drivers/net/ipa/ipa_uc.c
> +++ b/drivers/net/ipa/ipa_uc.c
> @@ -39,11 +39,12 @@
> #define IPA_SEND_DELAY 100 /* microseconds */
>
> /**
> - * struct ipa_uc_mem_area - AP/microcontroller shared memory area
> + * union ipa_uc_mem_area - AP/microcontroller shared memory area
> * @command: command code (AP->microcontroller)
> * @reserved0: reserved bytes; avoid reading or writing
> * @command_param: low 32 bits of command parameter (AP->microcontroller)
> * @command_param_hi: high 32 bits of command parameter (AP->microcontroller)
> + * Available since IPA v3.0
> *
> * @response: response code (microcontroller->AP)
> * @reserved1: reserved bytes; avoid reading or writing
> @@ -59,31 +60,58 @@
> * @reserved3: reserved bytes; avoid reading or writing
> * @interface_version: hardware-reported interface version
> * @reserved4: reserved bytes; avoid reading or writing
> + * @reserved5: reserved bytes; avoid reading or writing
> *
> * A shared memory area at the base of IPA resident memory is used for
> * communication with the microcontroller. The region is 128 bytes in
> * size, but only the first 40 bytes (structured this way) are used.
> */
> -struct ipa_uc_mem_area {
> - u8 command; /* enum ipa_uc_command */
> - u8 reserved0[3];
> - __le32 command_param;
> - __le32 command_param_hi;
> - u8 response; /* enum ipa_uc_response */
> - u8 reserved1[3];
> - __le32 response_param;
> - u8 event; /* enum ipa_uc_event */
> - u8 reserved2[3];
> -
> - __le32 event_param;
> - __le32 first_error_address;
> - u8 hw_state;
> - u8 warning_counter;
> - __le16 reserved3;
> - __le16 interface_version;
> - __le16 reserved4;
> +union ipa_uc_mem_area {
> + struct {
> + u8 command; /* enum ipa_uc_command */
> + u8 reserved0[3];
> + __le32 command_param;
> + u8 response; /* enum ipa_uc_response */
> + u8 reserved1[3];
> + __le32 response_param;
> + u8 event; /* enum ipa_uc_event */
> + u8 reserved2[3];
> +
> + __le32 event_param;
> + __le32 reserved3;
> + __le32 first_error_address;
> + u8 hw_state;
> + u8 warning_counter;
> + __le16 reserved4;
> + __le16 interface_version;
> + __le16 reserved5;
> + } v2;
> + struct {
> + u8 command; /* enum ipa_uc_command */
> + u8 reserved0[3];
> + __le32 command_param;
> + __le32 command_param_hi;
> + u8 response; /* enum ipa_uc_response */
> + u8 reserved1[3];
> + __le32 response_param;
> + u8 event; /* enum ipa_uc_event */
> + u8 reserved2[3];
> +
> + __le32 event_param;
> + __le32 first_error_address;
> + u8 hw_state;
> + u8 warning_counter;
> + __le16 reserved3;
> + __le16 interface_version;
> + __le16 reserved4;
> + } v3;
> };
>
> +#define UC_FIELD(_ipa, _field) \
> + *((_ipa->version >= IPA_VERSION_3_0) ? \
> + &(ipa_uc_shared(_ipa)->v3._field) : \
> + &(ipa_uc_shared(_ipa)->v2._field))
> +
> /** enum ipa_uc_command - commands from the AP to the microcontroller */
> enum ipa_uc_command {
> IPA_UC_COMMAND_NO_OP = 0x0,
> @@ -95,6 +123,7 @@ enum ipa_uc_command {
> IPA_UC_COMMAND_CLK_UNGATE = 0x6,
> IPA_UC_COMMAND_MEMCPY = 0x7,
> IPA_UC_COMMAND_RESET_PIPE = 0x8,
> + /* Next two commands are present for IPA v3.0+ */
> IPA_UC_COMMAND_REG_WRITE = 0x9,
> IPA_UC_COMMAND_GSI_CH_EMPTY = 0xa,
> };
> @@ -114,7 +143,7 @@ enum ipa_uc_event {
> IPA_UC_EVENT_LOG_INFO = 0x2,
> };
>
> -static struct ipa_uc_mem_area *ipa_uc_shared(struct ipa *ipa)
> +static union ipa_uc_mem_area *ipa_uc_shared(struct ipa *ipa)
> {
> const struct ipa_mem *mem = ipa_mem_find(ipa, IPA_MEM_UC_SHARED);
> u32 offset = ipa->mem_offset + mem->offset;
> @@ -125,22 +154,22 @@ static struct ipa_uc_mem_area *ipa_uc_shared(struct ipa *ipa)
> /* Microcontroller event IPA interrupt handler */
> static void ipa_uc_event_handler(struct ipa *ipa, enum ipa_irq_id irq_id)
> {
> - struct ipa_uc_mem_area *shared = ipa_uc_shared(ipa);
> struct device *dev = &ipa->pdev->dev;
> + u32 event = UC_FIELD(ipa, event);
>
> - if (shared->event == IPA_UC_EVENT_ERROR)
> + if (event == IPA_UC_EVENT_ERROR)
> dev_err(dev, "microcontroller error event\n");
> - else if (shared->event != IPA_UC_EVENT_LOG_INFO)
> + else if (event != IPA_UC_EVENT_LOG_INFO)
> dev_err(dev, "unsupported microcontroller event %u\n",
> - shared->event);
> + event);
> /* The LOG_INFO event can be safely ignored */
> }
>
> /* Microcontroller response IPA interrupt handler */
> static void ipa_uc_response_hdlr(struct ipa *ipa, enum ipa_irq_id irq_id)
> {
> - struct ipa_uc_mem_area *shared = ipa_uc_shared(ipa);
> struct device *dev = &ipa->pdev->dev;
> + u32 response = UC_FIELD(ipa, response);
>
> /* An INIT_COMPLETED response message is sent to the AP by the
> * microcontroller when it is operational. Other than this, the AP
> @@ -150,20 +179,21 @@ static void ipa_uc_response_hdlr(struct ipa *ipa, enum ipa_irq_id irq_id)
> * We can drop the power reference taken in ipa_uc_power() once we
> * know the microcontroller has finished its initialization.
> */
> - switch (shared->response) {
> + switch (response) {
> case IPA_UC_RESPONSE_INIT_COMPLETED:
> if (ipa->uc_powered) {
> ipa->uc_loaded = true;
> pm_runtime_mark_last_busy(dev);
> (void)pm_runtime_put_autosuspend(dev);
> ipa->uc_powered = false;
> + ipa_qmi_signal_uc_loaded(ipa);
> } else {
> dev_warn(dev, "unexpected init_completed response\n");
> }
> break;
> default:
> dev_warn(dev, "unsupported microcontroller response %u\n",
> - shared->response);
> + response);
> break;
> }
> }
> @@ -216,16 +246,16 @@ void ipa_uc_power(struct ipa *ipa)
> /* Send a command to the microcontroller */
> static void send_uc_command(struct ipa *ipa, u32 command, u32 command_param)
> {
> - struct ipa_uc_mem_area *shared = ipa_uc_shared(ipa);
> u32 offset;
> u32 val;
>
> /* Fill in the command data */
> - shared->command = command;
> - shared->command_param = cpu_to_le32(command_param);
> - shared->command_param_hi = 0;
> - shared->response = 0;
> - shared->response_param = 0;
> + UC_FIELD(ipa, command) = command;
> + UC_FIELD(ipa, command_param) = cpu_to_le32(command_param);
> + if (ipa->version >= IPA_VERSION_3_0)
> + ipa_uc_shared(ipa)->v3.command_param_hi = 1;
> + UC_FIELD(ipa, response) = 0;
> + UC_FIELD(ipa, response_param) = 0;
>
> /* Use an interrupt to tell the microcontroller the command is ready */
> val = u32_encode_bits(1, UC_INTR_FMASK);
>
Powered by blists - more mailing lists