mbox series

[v1,0/7] Add support for IPA v3.1, GSI v1.0, MSM8998 IPA

Message ID 20210211175015.200772-1-angelogioacchino.delregno@somainline.org
Headers show
Series Add support for IPA v3.1, GSI v1.0, MSM8998 IPA | expand

Message

AngeloGioacchino Del Regno Feb. 11, 2021, 5:50 p.m. UTC
Hey all!

This time around I thought that it would be nice to get some modem
action going on. We have it, it's working (ish), so just.. why not.

This series adds support for IPA v3.1 (featuring GSI v1.0) and also
takes account for some bits that are shared with other unimplemented
IPA v3 variants and it is specifically targeting MSM8998, for which
support is added.

Since the userspace isn't entirely ready (as far as I can see) for
data connection (3g/lte/whatever) through the modem, it was possible
to only partially test this series.
Specifically, loading the IPA firmware and setting up the interface
went just fine, along with a basic setup of the network interface
that got exposed by this driver.

With this series, the benefits that I see are:
 1. The modem doesn't crash anymore when trying to setup a data
    connection, as now the modem firmware seems to be happy with
    having IPA initialized and ready;
 2. Other random modem crashes while picking up LTE home network
    signal (even just for calling, nothing fancy) seem to be gone.

These are the reasons why I think that this series is ready for
upstream action. It's *at least* stabilizing the platform when
the modem is up.

This was tested on the F(x)Tec Pro 1 (MSM8998) smartphone.

AngeloGioacchino Del Regno (7):
  net: ipa: Add support for IPA v3.1 with GSI v1.0
  net: ipa: endpoint: Don't read unexistant register on IPAv3.1
  net: ipa: gsi: Avoid some writes during irq setup for older IPA
  net: ipa: gsi: Use right masks for GSI v1.0 channels hw param
  net: ipa: Add support for IPA on MSM8998
  dt-bindings: net: qcom-ipa: Document qcom,sc7180-ipa compatible
  dt-bindings: net: qcom-ipa: Document qcom,msm8998-ipa compatible

 .../devicetree/bindings/net/qcom,ipa.yaml     |   7 +-
 drivers/net/ipa/Makefile                      |   3 +-
 drivers/net/ipa/gsi.c                         |  33 +-
 drivers/net/ipa/gsi_reg.h                     |   5 +
 drivers/net/ipa/ipa_data-msm8998.c            | 407 ++++++++++++++++++
 drivers/net/ipa/ipa_data.h                    |   5 +
 drivers/net/ipa/ipa_endpoint.c                |  26 +-
 drivers/net/ipa/ipa_main.c                    |  12 +-
 drivers/net/ipa/ipa_reg.h                     |   3 +
 drivers/net/ipa/ipa_version.h                 |   1 +
 10 files changed, 480 insertions(+), 22 deletions(-)
 create mode 100644 drivers/net/ipa/ipa_data-msm8998.c

Comments

Alex Elder Feb. 11, 2021, 8:27 p.m. UTC | #1
On 2/11/21 11:50 AM, AngeloGioacchino Del Regno wrote:
> Hey all!
> 
> This time around I thought that it would be nice to get some modem
> action going on. We have it, it's working (ish), so just.. why not.

Thank you for the patches!

I would like to review these carefully but I'm sorry
I won't be able to get to it today, and possibly not
for a few days.  But I *will* review them.

I just want you to know I'm paying attention, though
I'm sort of buried in an important issue right now.

I'm very impressed at how small the patches are though.

					-Alex

> This series adds support for IPA v3.1 (featuring GSI v1.0) and also
> takes account for some bits that are shared with other unimplemented
> IPA v3 variants and it is specifically targeting MSM8998, for which
> support is added.
> 
> Since the userspace isn't entirely ready (as far as I can see) for
> data connection (3g/lte/whatever) through the modem, it was possible
> to only partially test this series.
> Specifically, loading the IPA firmware and setting up the interface
> went just fine, along with a basic setup of the network interface
> that got exposed by this driver.
> 
> With this series, the benefits that I see are:
>   1. The modem doesn't crash anymore when trying to setup a data
>      connection, as now the modem firmware seems to be happy with
>      having IPA initialized and ready;
>   2. Other random modem crashes while picking up LTE home network
>      signal (even just for calling, nothing fancy) seem to be gone.
> 
> These are the reasons why I think that this series is ready for
> upstream action. It's *at least* stabilizing the platform when
> the modem is up.
> 
> This was tested on the F(x)Tec Pro 1 (MSM8998) smartphone.
> 
> AngeloGioacchino Del Regno (7):
>    net: ipa: Add support for IPA v3.1 with GSI v1.0
>    net: ipa: endpoint: Don't read unexistant register on IPAv3.1
>    net: ipa: gsi: Avoid some writes during irq setup for older IPA
>    net: ipa: gsi: Use right masks for GSI v1.0 channels hw param
>    net: ipa: Add support for IPA on MSM8998
>    dt-bindings: net: qcom-ipa: Document qcom,sc7180-ipa compatible
>    dt-bindings: net: qcom-ipa: Document qcom,msm8998-ipa compatible
> 
>   .../devicetree/bindings/net/qcom,ipa.yaml     |   7 +-
>   drivers/net/ipa/Makefile                      |   3 +-
>   drivers/net/ipa/gsi.c                         |  33 +-
>   drivers/net/ipa/gsi_reg.h                     |   5 +
>   drivers/net/ipa/ipa_data-msm8998.c            | 407 ++++++++++++++++++
>   drivers/net/ipa/ipa_data.h                    |   5 +
>   drivers/net/ipa/ipa_endpoint.c                |  26 +-
>   drivers/net/ipa/ipa_main.c                    |  12 +-
>   drivers/net/ipa/ipa_reg.h                     |   3 +
>   drivers/net/ipa/ipa_version.h                 |   1 +
>   10 files changed, 480 insertions(+), 22 deletions(-)
>   create mode 100644 drivers/net/ipa/ipa_data-msm8998.c
>
AngeloGioacchino Del Regno Feb. 11, 2021, 9:51 p.m. UTC | #2
Il 11/02/21 21:27, Alex Elder ha scritto:
> On 2/11/21 11:50 AM, AngeloGioacchino Del Regno wrote:
>> Hey all!
>>
>> This time around I thought that it would be nice to get some modem
>> action going on. We have it, it's working (ish), so just.. why not.
> 
> Thank you for the patches!
> 
> I would like to review these carefully but I'm sorry
> I won't be able to get to it today, and possibly not
> for a few days.  But I *will* review them.
> 

Don't worry :))

> I just want you to know I'm paying attention, though
> I'm sort of buried in an important issue right now.
> 
> I'm very impressed at how small the patches are though.

Actually, the driver is in a great shape. That's why the patches are
that small: thanks to you!

-- Angelo

> 
>                      -Alex
> 
>> This series adds support for IPA v3.1 (featuring GSI v1.0) and also
>> takes account for some bits that are shared with other unimplemented
>> IPA v3 variants and it is specifically targeting MSM8998, for which
>> support is added.
>>
>> Since the userspace isn't entirely ready (as far as I can see) for
>> data connection (3g/lte/whatever) through the modem, it was possible
>> to only partially test this series.
>> Specifically, loading the IPA firmware and setting up the interface
>> went just fine, along with a basic setup of the network interface
>> that got exposed by this driver.
>>
>> With this series, the benefits that I see are:
>>   1. The modem doesn't crash anymore when trying to setup a data
>>      connection, as now the modem firmware seems to be happy with
>>      having IPA initialized and ready;
>>   2. Other random modem crashes while picking up LTE home network
>>      signal (even just for calling, nothing fancy) seem to be gone.
>>
>> These are the reasons why I think that this series is ready for
>> upstream action. It's *at least* stabilizing the platform when
>> the modem is up.
>>
>> This was tested on the F(x)Tec Pro 1 (MSM8998) smartphone.
>>
>> AngeloGioacchino Del Regno (7):
>>    net: ipa: Add support for IPA v3.1 with GSI v1.0
>>    net: ipa: endpoint: Don't read unexistant register on IPAv3.1
>>    net: ipa: gsi: Avoid some writes during irq setup for older IPA
>>    net: ipa: gsi: Use right masks for GSI v1.0 channels hw param
>>    net: ipa: Add support for IPA on MSM8998
>>    dt-bindings: net: qcom-ipa: Document qcom,sc7180-ipa compatible
>>    dt-bindings: net: qcom-ipa: Document qcom,msm8998-ipa compatible
>>
>>   .../devicetree/bindings/net/qcom,ipa.yaml     |   7 +-
>>   drivers/net/ipa/Makefile                      |   3 +-
>>   drivers/net/ipa/gsi.c                         |  33 +-
>>   drivers/net/ipa/gsi_reg.h                     |   5 +
>>   drivers/net/ipa/ipa_data-msm8998.c            | 407 ++++++++++++++++++
>>   drivers/net/ipa/ipa_data.h                    |   5 +
>>   drivers/net/ipa/ipa_endpoint.c                |  26 +-
>>   drivers/net/ipa/ipa_main.c                    |  12 +-
>>   drivers/net/ipa/ipa_reg.h                     |   3 +
>>   drivers/net/ipa/ipa_version.h                 |   1 +
>>   10 files changed, 480 insertions(+), 22 deletions(-)
>>   create mode 100644 drivers/net/ipa/ipa_data-msm8998.c
>>
>
Jakub Kicinski Feb. 12, 2021, 6:49 p.m. UTC | #3
On Thu, 11 Feb 2021 18:50:09 +0100 AngeloGioacchino Del Regno wrote:
> In preparation for adding support for the MSM8998 SoC's IPA,
> add the necessary bits for IPA version 3.1 featuring GSI 1.0,
> found on at least MSM8998.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>

please check your patches with ./scripts/kernel-doc for warnings:

drivers/net/ipa/ipa_version.h:24: warning: Enum value 'IPA_VERSION_3_1' not described in enum 'ipa_version'
Alex Elder March 2, 2021, 2:04 a.m. UTC | #4
On 2/11/21 11:50 AM, AngeloGioacchino Del Regno wrote:
> Hey all!
> 
> This time around I thought that it would be nice to get some modem
> action going on. We have it, it's working (ish), so just.. why not.
> 
> This series adds support for IPA v3.1 (featuring GSI v1.0) and also
> takes account for some bits that are shared with other unimplemented
> IPA v3 variants and it is specifically targeting MSM8998, for which
> support is added.

It was more like "next month" rather than "next week," but I
finally took some more time to look at this today.

Again I think it's surprising how little code you had
to implement to get something that seems is at least
modestly functional.

FYI I have undertaken an effort to make the upstream code
suitable for use for any IPA version (3.0-4.11) in the
past few months.  Most of what I've done is in line with
the things you found were necessary for IPA v3.1 support.
Early on I got most of the support for IPA v4.5 upstream,
and have been holding off trying to get other similar
changes out for review for other versions until I've had
more of a chance to test some of what's new in IPA v4.5.

In the coming weeks I will start posting more of this
work for review.  You'll see that I'm modifying many
things you do in your series (such as making version
checks not assume only v3.5.1 and v4.2 are supported).
My priority is on newer versions, but I want the code
to be (at least) correct for IPA v3.0, v3.1, and v3.5
as well.

What might be best is for you to consider using the
patches when I send them out.  I'll gladly give you some
credit when I do if you like (suggested-by, reviewed-by,
tested-by, whatever you feel is appropriate).  Please
let me know if you would like to be on the Cc list for
this sort of change.

> Since the userspace isn't entirely ready (as far as I can see) for
> data connection (3g/lte/whatever) through the modem, it was possible
> to only partially test this series.

Yes we're still figuring out how the upstream tools need
to interact with the kernel for configuration.

> Specifically, loading the IPA firmware and setting up the interface
> went just fine, along with a basic setup of the network interface
> that got exposed by this driver.

This is great to hear.

> With this series, the benefits that I see are:
>  1. The modem doesn't crash anymore when trying to setup a data
>     connection, as now the modem firmware seems to be happy with
>     having IPA initialized and ready;
>  2. Other random modem crashes while picking up LTE home network
>     signal (even just for calling, nothing fancy) seem to be gone.
> 
> These are the reasons why I think that this series is ready for
> upstream action. It's *at least* stabilizing the platform when
> the modem is up.
> 
> This was tested on the F(x)Tec Pro 1 (MSM8998) smartphone.

I unfortunately can't promise you you'll have the full
connection up and running, but we can probably get very
close.

It would be very helpful for you (someone other than me,
that is) to participate in validating the changes I am
now finalizing.  I hope you're willing.

I'll offer a few more specific comments on each of your
patches.

					-Alex


> AngeloGioacchino Del Regno (7):
>   net: ipa: Add support for IPA v3.1 with GSI v1.0
>   net: ipa: endpoint: Don't read unexistant register on IPAv3.1
>   net: ipa: gsi: Avoid some writes during irq setup for older IPA
>   net: ipa: gsi: Use right masks for GSI v1.0 channels hw param
>   net: ipa: Add support for IPA on MSM8998
>   dt-bindings: net: qcom-ipa: Document qcom,sc7180-ipa compatible
>   dt-bindings: net: qcom-ipa: Document qcom,msm8998-ipa compatible
> 
>  .../devicetree/bindings/net/qcom,ipa.yaml     |   7 +-
>  drivers/net/ipa/Makefile                      |   3 +-
>  drivers/net/ipa/gsi.c                         |  33 +-
>  drivers/net/ipa/gsi_reg.h                     |   5 +
>  drivers/net/ipa/ipa_data-msm8998.c            | 407 ++++++++++++++++++
>  drivers/net/ipa/ipa_data.h                    |   5 +
>  drivers/net/ipa/ipa_endpoint.c                |  26 +-
>  drivers/net/ipa/ipa_main.c                    |  12 +-
>  drivers/net/ipa/ipa_reg.h                     |   3 +
>  drivers/net/ipa/ipa_version.h                 |   1 +
>  10 files changed, 480 insertions(+), 22 deletions(-)
>  create mode 100644 drivers/net/ipa/ipa_data-msm8998.c
>