2018-06-12 12:32:49

by Govind Singh

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

On 2018-06-12 17:45, Govind Singh wrote:
> -----Original Message-----
> From: ath10k <[email protected]> On Behalf Of Niklas
> Cassel
> Sent: Tuesday, June 12, 2018 5:09 PM
> To: Kalle Valo <[email protected]>; David S. Miller
> <[email protected]>
> Cc: Niklas Cassel <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST
>
> ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
>
> Signed-off-by: Niklas Cassel <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
> b/drivers/net/wireless/ath/ath10k/Kconfig
> index 54ff5930126c..6572a43590a8 100644
> --- a/drivers/net/wireless/ath/ath10k/Kconfig
> +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> @@ -42,7 +42,8 @@ config ATH10K_USB
>
> config ATH10K_SNOC
> tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
> - depends on ATH10K && ARCH_QCOM
> + depends on ATH10K
> + depends on ARCH_QCOM || COMPILE_TEST
> ---help---
> This module adds support for integrated WCN3990 chip connected
> to system NOC(SNOC). Currently work in progress and will not

Thanks Niklas for enabling COMPILE_TEST. With QMI set of
changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
COMPILE_TEST for
QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix
the same.

config QCOM_SCM
bool
- depends on ARM || ARM64
+ depends on ARM || ARM64 || COMPILE_TEST
select RESET_CONTROLLER


config QCOM_SCM_64
def_bool y
- depends on QCOM_SCM && ARM64
+ depends on QCOM_SCM && ARM64 || COMPILE_TEST

config QCOM_QMI_HELPERS
tristate
- depends on ARCH_QCOM && NET
+ depends on (ARCH_QCOM || COMPILE_TEST) && NET

-obj-$(CONFIG_ARCH_QCOM) += qcom/
+obj-y += qcom/

__qcom_scm_init/qcom_scm_call wrapper to support COMPILE_TEST.



BR,
Govind


2018-06-13 13:28:23

by Niklas Cassel

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

On Tue, Jun 12, 2018 at 02:44:03PM +0200, Niklas Cassel wrote:
> On Tue, Jun 12, 2018 at 06:02:48PM +0530, Govind Singh wrote:
> > On 2018-06-12 17:45, Govind Singh wrote:
> > > -----Original Message-----
> > > From: ath10k <[email protected]> On Behalf Of Niklas
> > > Cassel
> > > Sent: Tuesday, June 12, 2018 5:09 PM
> > > To: Kalle Valo <[email protected]>; David S. Miller
> > > <[email protected]>
> > > Cc: Niklas Cassel <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Subject: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST
> > >
> > > ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
> > >
> > > Signed-off-by: Niklas Cassel <[email protected]>
> > > ---
> > > drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
> > > b/drivers/net/wireless/ath/ath10k/Kconfig
> > > index 54ff5930126c..6572a43590a8 100644
> > > --- a/drivers/net/wireless/ath/ath10k/Kconfig
> > > +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> > > @@ -42,7 +42,8 @@ config ATH10K_USB
> > >
> > > config ATH10K_SNOC
> > > tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
> > > - depends on ATH10K && ARCH_QCOM
> > > + depends on ATH10K
> > > + depends on ARCH_QCOM || COMPILE_TEST
> > > ---help---
> > > This module adds support for integrated WCN3990 chip connected
> > > to system NOC(SNOC). Currently work in progress and will not
> >
> > Thanks Niklas for enabling COMPILE_TEST. With QMI set of
> > changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
> > COMPILE_TEST for
> > QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix the
> > same.
>
>

This patch is good as is.

However, Govind's QMI patch set together with this patch
resulted in build errors.

FTR, these are fixed by:
https://marc.info/?l=linux-kernel&m=152880985402356
https://marc.info/?l=linux-kernel&m=152889452326350


Regards,
Niklas

2018-06-12 12:44:07

by Niklas Cassel

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

On Tue, Jun 12, 2018 at 06:02:48PM +0530, Govind Singh wrote:
> On 2018-06-12 17:45, Govind Singh wrote:
> > -----Original Message-----
> > From: ath10k <[email protected]> On Behalf Of Niklas
> > Cassel
> > Sent: Tuesday, June 12, 2018 5:09 PM
> > To: Kalle Valo <[email protected]>; David S. Miller
> > <[email protected]>
> > Cc: Niklas Cassel <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST
> >
> > ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
> >
> > Signed-off-by: Niklas Cassel <[email protected]>
> > ---
> > drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
> > b/drivers/net/wireless/ath/ath10k/Kconfig
> > index 54ff5930126c..6572a43590a8 100644
> > --- a/drivers/net/wireless/ath/ath10k/Kconfig
> > +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> > @@ -42,7 +42,8 @@ config ATH10K_USB
> >
> > config ATH10K_SNOC
> > tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
> > - depends on ATH10K && ARCH_QCOM
> > + depends on ATH10K
> > + depends on ARCH_QCOM || COMPILE_TEST
> > ---help---
> > This module adds support for integrated WCN3990 chip connected
> > to system NOC(SNOC). Currently work in progress and will not
>
> Thanks Niklas for enabling COMPILE_TEST. With QMI set of
> changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
> COMPILE_TEST for
> QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix the
> same.


Argh..

qcom_scm seems fine, it is just missing a single definition in the
#else clause of include/linux/qcom_scm.h.

+++ b/include/linux/qcom_scm.h
@@ -89,6 +89,10 @@ static inline int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr,
static inline int
qcom_scm_pas_auth_and_reset(u32 peripheral) { return -ENODEV; }
static inline int qcom_scm_pas_shutdown(u32 peripheral) { return -ENODEV; }
+static inline int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
+ unsigned int *src,
+ struct qcom_scm_vmperm *newvm,
+ int dest_cnt) { return -ENODEV; }
static inline void qcom_scm_cpu_power_down(u32 flags) {}
static inline u32 qcom_scm_get_version(void) { return 0; }



include/linux/soc/qcom/qmi.h on the other hand doesn't have any
dummy defintions at all.
I think that it makes sense to be able to compile test
the QMI helpers also on other archs..

Bjorn, any opinion?


>
> config QCOM_SCM
> bool
> - depends on ARM || ARM64
> + depends on ARM || ARM64 || COMPILE_TEST
> select RESET_CONTROLLER
>
>
> config QCOM_SCM_64
> def_bool y
> - depends on QCOM_SCM && ARM64
> + depends on QCOM_SCM && ARM64 || COMPILE_TEST
>
> config QCOM_QMI_HELPERS
> tristate
> - depends on ARCH_QCOM && NET
> + depends on (ARCH_QCOM || COMPILE_TEST) && NET
>
> -obj-$(CONFIG_ARCH_QCOM) += qcom/
> +obj-y += qcom/
>
> __qcom_scm_init/qcom_scm_call wrapper to support COMPILE_TEST.
>
>
>
> BR,
> Govind

2018-06-13 08:47:35

by Kalle Valo

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

Niklas Cassel <[email protected]> writes:

> On Tue, Jun 12, 2018 at 06:02:48PM +0530, Govind Singh wrote:
>> On 2018-06-12 17:45, Govind Singh wrote:
>> > -----Original Message-----
>> > From: ath10k <[email protected]> On Behalf Of Niklas
>> > Cassel
>> > Sent: Tuesday, June 12, 2018 5:09 PM
>> > To: Kalle Valo <[email protected]>; David S. Miller
>> > <[email protected]>
>> > Cc: Niklas Cassel <[email protected]>; [email protected];
>> > [email protected]; [email protected];
>> > [email protected]
>> > Subject: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST
>> >
>> > ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
>> >
>> > Signed-off-by: Niklas Cassel <[email protected]>
>> > ---
>> > drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
>> > b/drivers/net/wireless/ath/ath10k/Kconfig
>> > index 54ff5930126c..6572a43590a8 100644
>> > --- a/drivers/net/wireless/ath/ath10k/Kconfig
>> > +++ b/drivers/net/wireless/ath/ath10k/Kconfig
>> > @@ -42,7 +42,8 @@ config ATH10K_USB
>> >
>> > config ATH10K_SNOC
>> > tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
>> > - depends on ATH10K && ARCH_QCOM
>> > + depends on ATH10K
>> > + depends on ARCH_QCOM || COMPILE_TEST
>> > ---help---
>> > This module adds support for integrated WCN3990 chip connected
>> > to system NOC(SNOC). Currently work in progress and will not
>>
>> Thanks Niklas for enabling COMPILE_TEST. With QMI set of
>> changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
>> COMPILE_TEST for
>> QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix the
>> same.
>
>
> Argh..
>
> qcom_scm seems fine, it is just missing a single definition in the
> #else clause of include/linux/qcom_scm.h.
>
> +++ b/include/linux/qcom_scm.h
> @@ -89,6 +89,10 @@ static inline int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr,
> static inline int
> qcom_scm_pas_auth_and_reset(u32 peripheral) { return -ENODEV; }
> static inline int qcom_scm_pas_shutdown(u32 peripheral) { return -ENODEV; }
> +static inline int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
> + unsigned int *src,
> + struct qcom_scm_vmperm *newvm,
> + int dest_cnt) { return -ENODEV; }
> static inline void qcom_scm_cpu_power_down(u32 flags) {}
> static inline u32 qcom_scm_get_version(void) { return 0; }
>
>
>
> include/linux/soc/qcom/qmi.h on the other hand doesn't have any
> dummy defintions at all.
> I think that it makes sense to be able to compile test
> the QMI helpers also on other archs..
>
> Bjorn, any opinion?

Please don't drop ath10k list, adding it back.

--
Kalle Valo

2018-06-14 14:09:11

by Kalle Valo

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

Niklas Cassel <[email protected]> writes:

> On Tue, Jun 12, 2018 at 02:44:03PM +0200, Niklas Cassel wrote:
>> On Tue, Jun 12, 2018 at 06:02:48PM +0530, Govind Singh wrote:
>> > On 2018-06-12 17:45, Govind Singh wrote:
>> > >
>> > > ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
>> > >
>> > > Signed-off-by: Niklas Cassel <[email protected]>
>> > > ---
>> > > drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
>> > > 1 file changed, 2 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
>> > > b/drivers/net/wireless/ath/ath10k/Kconfig
>> > > index 54ff5930126c..6572a43590a8 100644
>> > > --- a/drivers/net/wireless/ath/ath10k/Kconfig
>> > > +++ b/drivers/net/wireless/ath/ath10k/Kconfig
>> > > @@ -42,7 +42,8 @@ config ATH10K_USB
>> > >
>> > > config ATH10K_SNOC
>> > > tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
>> > > - depends on ATH10K && ARCH_QCOM
>> > > + depends on ATH10K
>> > > + depends on ARCH_QCOM || COMPILE_TEST
>> > > ---help---
>> > > This module adds support for integrated WCN3990 chip connected
>> > > to system NOC(SNOC). Currently work in progress and will not
>> >
>> > Thanks Niklas for enabling COMPILE_TEST. With QMI set of
>> > changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
>> > COMPILE_TEST for
>> > QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix the
>> > same.
>
> This patch is good as is.
>
> However, Govind's QMI patch set together with this patch
> resulted in build errors.
>
> FTR, these are fixed by:
> https://marc.info/?l=linux-kernel&m=152880985402356
> https://marc.info/?l=linux-kernel&m=152889452326350

So the problem is that if I apply this patch I can't apply Govind's QMI
patchset (due to the build problems) until Niklas' fixes to qcom and
rpmsg subsystems propogate back to my tree and that might take weeks, or
even months. But I really would like to apply the QMI patchset ASAP so
that we can complete the wcn3990 support and not unnecessarily delay it.

So what I propose is that I put this patch 2 as 'Awaiting Upstream' in
patchwork and apply it once Niklas' patches get to my tree. Does that
sound good?

--
Kalle Valo

2018-06-14 18:42:53

by Niklas Cassel

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] ath10k: allow ATH10K_SNOC with COMPILE_TEST

On Thu, Jun 14, 2018 at 05:09:04PM +0300, Kalle Valo wrote:
> Niklas Cassel <[email protected]> writes:
>
> > On Tue, Jun 12, 2018 at 02:44:03PM +0200, Niklas Cassel wrote:
> >> On Tue, Jun 12, 2018 at 06:02:48PM +0530, Govind Singh wrote:
> >> > On 2018-06-12 17:45, Govind Singh wrote:
> >> > >
> >> > > ATH10K_SNOC builds just fine with COMPILE_TEST, so make that possible.
> >> > >
> >> > > Signed-off-by: Niklas Cassel <[email protected]>
> >> > > ---
> >> > > drivers/net/wireless/ath/ath10k/Kconfig | 3 ++-
> >> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig
> >> > > b/drivers/net/wireless/ath/ath10k/Kconfig
> >> > > index 54ff5930126c..6572a43590a8 100644
> >> > > --- a/drivers/net/wireless/ath/ath10k/Kconfig
> >> > > +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> >> > > @@ -42,7 +42,8 @@ config ATH10K_USB
> >> > >
> >> > > config ATH10K_SNOC
> >> > > tristate "Qualcomm ath10k SNOC support (EXPERIMENTAL)"
> >> > > - depends on ATH10K && ARCH_QCOM
> >> > > + depends on ATH10K
> >> > > + depends on ARCH_QCOM || COMPILE_TEST
> >> > > ---help---
> >> > > This module adds support for integrated WCN3990 chip connected
> >> > > to system NOC(SNOC). Currently work in progress and will not
> >> >
> >> > Thanks Niklas for enabling COMPILE_TEST. With QMI set of
> >> > changes(https://patchwork.kernel.org/patch/10448183/), we need to enable
> >> > COMPILE_TEST for
> >> > QCOM_SCM/QMI_HELPERS which seems broken today. Are you planning to fix the
> >> > same.
> >
> > This patch is good as is.
> >
> > However, Govind's QMI patch set together with this patch
> > resulted in build errors.
> >
> > FTR, these are fixed by:
> > https://marc.info/?l=linux-kernel&m=152880985402356
> > https://marc.info/?l=linux-kernel&m=152889452326350
>
> So the problem is that if I apply this patch I can't apply Govind's QMI
> patchset (due to the build problems) until Niklas' fixes to qcom and
> rpmsg subsystems propogate back to my tree and that might take weeks, or
> even months. But I really would like to apply the QMI patchset ASAP so
> that we can complete the wcn3990 support and not unnecessarily delay it.
>
> So what I propose is that I put this patch 2 as 'Awaiting Upstream' in
> patchwork and apply it once Niklas' patches get to my tree. Does that
> sound good?

Absolutely.

I didn't realize this until after sending the patch.


Kind regards,
Niklas