Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Dan Carpenter 
On Wed, Mar 11, 2020 at 07:08:11PM +0530, Shreeya Patel wrote:
> diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c 
> b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> index e813382e78a6..643592b0bd38 100644
> --- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> +++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> @@ -551,18 +551,11 @@ static void HalRxAggr8723BSdio(struct adapter *padapter)
>  
>   pregistrypriv = &padapter->registrypriv;
>  
> - if (pregistrypriv->wifi_spec) {
> - /*  2010.04.27 hpfan */
> - /*  Adjust RxAggrTimeout to close to zero disable RxAggr, 
> suggested by designer */
> - /*  Timeout value is calculated by 34 / (2^n) */
> - valueDMATimeout = 0x06;
> - valueDMAPageCount = 0x06;
> - } else {
> - /*  20130530, Isaac@SD1 suggest 3 kinds of parameter */
> - /*  TX/RX Balance */
> - valueDMATimeout = 0x06;
> - valueDMAPageCount = 0x06;
> - }
> + /*  2010.04.27 hpfan */

Delete these sorts of comments where it's just a name of someone and
a time stamp when they wrote it.  We don't know how to contact "hpfan"
so it's useless.

regards,
dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Shreeya Patel 
On Fri, 2020-03-13 at 10:20 +0300, Dan Carpenter wrote:
> On Wed, Mar 11, 2020 at 07:08:11PM +0530, Shreeya Patel wrote:
> > diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> > b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> > index e813382e78a6..643592b0bd38 100644
> > --- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> > +++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
> > @@ -551,18 +551,11 @@ static void HalRxAggr8723BSdio(struct adapter
> > *padapter)
> >  
> > pregistrypriv = &padapter->registrypriv;
> >  
> > -   if (pregistrypriv->wifi_spec) {
> > -   /*  2010.04.27 hpfan */
> > -   /*  Adjust RxAggrTimeout to close to zero disable
> > RxAggr, suggested by designer */
> > -   /*  Timeout value is calculated by 34 / (2^n) */
> > -   valueDMATimeout = 0x06;
> > -   valueDMAPageCount = 0x06;
> > -   } else {
> > -   /*  20130530, Isaac@SD1 suggest 3 kinds of parameter */
> > -   /*  TX/RX Balance */
> > -   valueDMATimeout = 0x06;
> > -   valueDMAPageCount = 0x06;
> > -   }
> > +   /*  2010.04.27 hpfan */
> 
> Delete these sorts of comments where it's just a name of someone and
> a time stamp when they wrote it.  We don't know how to contact
> "hpfan"
> so it's useless.
> 

Thanks Joe and Dan for your explanation. I will remove the comments and
send the patch again.

> regards,
> dan carpenter
> 
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: rtw_mlme: Remove unnecessary conditions

2020-03-13 Thread Dan Carpenter 
On Wed, Mar 11, 2020 at 07:28:59PM +0530, Shreeya Patel wrote:
> Remove unnecessary if and else conditions since both are leading to the
> initialization of "phtpriv->ampdu_enable" with the same value.
> 
> Signed-off-by: Shreeya Patel 
> ---
>  drivers/staging/rtl8723bs/core/rtw_mlme.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 71fcb466019a..48e9faf27321 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -2772,13 +2772,9 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 
> *pie, uint ie_len, u8 channe
>  
>   /* maybe needs check if ap supports rx ampdu. */
>   if (!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == 1) {
> - if (pregistrypriv->wifi_spec == 1) {
> - /* remove this part because testbed AP should disable 
> RX AMPDU */
> - /* phtpriv->ampdu_enable = false; */
> - phtpriv->ampdu_enable = true;
> - } else {
> - phtpriv->ampdu_enable = true;
> - }
> + /* remove this part because testbed AP should disable RX AMPDU 
> */
> + /* phtpriv->ampdu_enable = false; */

Please delete this dead code and the related comment.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH] Staging: rtl8723bs: rtw_mlme: Remove unnecessary conditions

2020-03-13 Thread Dan Carpenter 
The original patch description was basically fine.  Outreachy reviews
tend to be more pedantic about this sort of stuff than most maintainers.
There are a couple who have very strict rules, but try to avoid those
maintainers and your life will be happier.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls

2020-03-13 Thread Hans Verkuil 
Hi Alex,

I apologize for the (very) slow reply, but better late than never.

On 1/13/20 3:15 PM, Alex Riesen wrote:
> This change implements audio-related V4L2 ioctls for the HDMI subdevice.

This is really where things go wrong. These V4L2 audio ioctls are meant for
old PCI TV tuner devices where the audio was implemented as audio jack outputs
that are typically looped back to audio inputs on a (PCI) soundcard. And when
these ioctls were designed ALSA didn't even exist.

None of that applies here.

Generally an hdmi driver will configure the i2s audio automatically, which is
typically connected to the SoC and controlled by the ALSA driver of the SoC,
but there may well be missing features (audio never got a lot of attention in
hdmi receivers). So what I would like to know is: what features are missing?

Anything missing can likely be resolved by adding HDMI audio specific V4L2 
controls,
which would be the right approach for this.

So I would expect to see a proposal for V4L2_CID_DV_RX_AUDIO_ controls to be
added here:

https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-dv.html

Regards,

Hans

> 
> The master audio clock is configured for 256fs, as supported by the only
> device available at the moment. For the same reason, the TDM slot is
> formatted using left justification of its bits.
> 
> Signed-off-by: Alexander Riesen 
> ---
>  drivers/media/i2c/adv748x/adv748x-core.c |   6 +
>  drivers/media/i2c/adv748x/adv748x-hdmi.c | 182 +++
>  drivers/media/i2c/adv748x/adv748x.h  |  42 ++
>  3 files changed, 230 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
> b/drivers/media/i2c/adv748x/adv748x-core.c
> index bc49aa93793c..b6067ffb1e0d 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -150,6 +150,12 @@ static int adv748x_write_check(struct adv748x_state 
> *state, u8 page, u8 reg,
>   return *error;
>  }
>  
> +int adv748x_update_bits(struct adv748x_state *state, u8 page, u8 reg, u8 
> mask,
> + u8 value)
> +{
> + return regmap_update_bits(state->regmap[page], reg, mask, value);
> +}
> +
>  /* adv748x_write_block(): Write raw data with a maximum of 
> I2C_SMBUS_BLOCK_MAX
>   * size to one or more registers.
>   *
> diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c 
> b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> index c557f8fdf11a..9bc9237c9116 100644
> --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2017 Renesas Electronics Corp.
>   */
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -603,11 +604,186 @@ static const struct v4l2_subdev_pad_ops 
> adv748x_pad_ops_hdmi = {
>   .enum_dv_timings = adv748x_hdmi_enum_dv_timings,
>  };
>  
> +static int adv748x_hdmi_audio_mute(struct adv748x_hdmi *hdmi, int enable)
> +{
> + struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
> +
> + return hdmi_update(state, ADV748X_HDMI_MUTE_CTRL,
> +ADV748X_HDMI_MUTE_CTRL_MUTE_AUDIO,
> +enable ? 0xff : 0);
> +}
> +
> +
> +#define HDMI_AOUT_NONE 0
> +#define HDMI_AOUT_I2S 1
> +#define HDMI_AOUT_I2S_TDM 2
> +
> +static int adv748x_hdmi_enumaudout(struct adv748x_hdmi *hdmi,
> +struct v4l2_audioout *a)
> +{
> + switch (a->index) {
> + case HDMI_AOUT_NONE:
> + strlcpy(a->name, "None", sizeof(a->name));
> + break;
> + case HDMI_AOUT_I2S:
> + strlcpy(a->name, "I2S/stereo", sizeof(a->name));
> + break;
> + case HDMI_AOUT_I2S_TDM:
> + strlcpy(a->name, "I2S-TDM/multichannel", sizeof(a->name));
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int adv748x_hdmi_g_audout(struct adv748x_hdmi *hdmi,
> +  struct v4l2_audioout *a)
> +{
> + a->index = hdmi->audio_out;
> + return adv748x_hdmi_enumaudout(hdmi, a);
> +}
> +
> +static int set_audio_pads_state(struct adv748x_state *state, int on)
> +{
> + return io_update(state, ADV748X_IO_PAD_CONTROLS,
> +  ADV748X_IO_PAD_CONTROLS_TRI_AUD |
> +  ADV748X_IO_PAD_CONTROLS_PDN_AUD,
> +  on ? 0 : 0xff);
> +}
> +
> +static int set_dpll_mclk_fs(struct adv748x_state *state, int fs)
> +{
> + if (fs % 128 || fs > 768)
> + return -EINVAL;
> + return dpll_update(state, ADV748X_DPLL_MCLK_FS,
> +ADV748X_DPLL_MCLK_FS_N_MASK, (fs / 128) - 1);
> +}
> +
> +static int set_i2s_format(struct adv748x_state *state, uint outmode,
> +   uint bitwidth)
> +{
> + return hdmi_update(state, ADV748X_HDMI_I2S,
> +ADV748X_HDMI_I2SBITWIDTH_MASK |
> +ADV748X_HDMI_I2SOUTMODE_MASK,
> +(outmode << ADV748X_HDM

Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio

2020-03-13 Thread Hans Verkuil 
Hi Alex,

Again, sorry for the late reply.

Patch 2/8 has its own comment since that approach won't work.

As a general note for this series: it might be better to have two
patch series: one for patches 1 and 3-6 (not sure whether 5 can be included
or not), and one where the public API changes (i.e. new V4L2 audio controls)
are added. The first can probably be merged fairly quickly, the second will
likely require more iterations since public API patches always take much longer
before they are mature.

Regards,

Hans

On 1/13/20 3:14 PM, Alex Riesen wrote:
> This adds minimal support for controlling the audio output I2S port available
> on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio
> signal from the decoded HDMI stream.
> 
> An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of this
> code.
> 
> Alex Riesen (8):
>   media: adv748x: add a device-specific wrapper for register block read
>   media: adv748x: add audio mute control and output selection ioctls
>   media: adv748x: add log_status ioctl
>   media: adv748x: reserve space for the audio (I2S) port in the driver
> structures
>   media: adv748x: add an ASoC DAI definition to the driver
>   media: adv748x: reduce amount of code for bitwise modification of
> device registers
>   dt-bindings: adv748x: add information about serial audio interface
> (I2S/TDM)
>   arm64: dts: renesas: salvator: add a connection from adv748x codec
> (HDMI input) to the R-Car SoC
> 
>  .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
>  .../dts/renesas/r8a7795-es1-salvator-x.dts|  24 +-
>  .../boot/dts/renesas/salvator-common.dtsi |  35 +-
>  drivers/media/i2c/adv748x/adv748x-core.c  |  54 +++
>  drivers/media/i2c/adv748x/adv748x-hdmi.c  | 355 ++
>  drivers/media/i2c/adv748x/adv748x.h   |  53 ++-
>  6 files changed, 523 insertions(+), 11 deletions(-)
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio

2020-03-13 Thread Alex Riesen 
Hi Hans,

Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:
> As a general note for this series: it might be better to have two
> patch series: one for patches 1 and 3-6 (not sure whether 5 can be included
> or not), and one where the public API changes (i.e. new V4L2 audio controls)
> are added. The first can probably be merged fairly quickly, the second will
> likely require more iterations since public API patches always take much 
> longer
> before they are mature.

I see. After the discussion started, I started to have suspicions of my own
regarding the V4L2 ioctls. Except for log-status, which is a practical
diagnostics feature (even supported by v4l2-ctl), I'm thinking about dropping
them altogether in favor of audio soc DAI implementation.
The DAI implementation does all we ever needed from the device. Besides,
selecting a I2S protocol variant from user space (I2S vs I2S/TDM) never felt
right.

Shall I submit the log-status separately?

Regards,
Alex

> On 1/13/20 3:14 PM, Alex Riesen wrote:
> > This adds minimal support for controlling the audio output I2S port 
> > available
> > on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio
> > signal from the decoded HDMI stream.
> > 
> > An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of 
> > this
> > code.
> > 
> > Alex Riesen (8):
> >  1. media: adv748x: add a device-specific wrapper for register block read
> >  2. media: adv748x: add audio mute control and output selection ioctls
> >  3. media: adv748x: add log_status ioctl
> >  4. media: adv748x: reserve space for the audio (I2S) port in the driver
> > structures
> >  5. media: adv748x: add an ASoC DAI definition to the driver
> >  6. media: adv748x: reduce amount of code for bitwise modification of
> > device registers
> >  7. dt-bindings: adv748x: add information about serial audio interface
> > (I2S/TDM)
> >  8. arm64: dts: renesas: salvator: add a connection from adv748x codec
> > (HDMI input) to the R-Car SoC
> > 
> >  .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
> >  .../dts/renesas/r8a7795-es1-salvator-x.dts|  24 +-
> >  .../boot/dts/renesas/salvator-common.dtsi |  35 +-
> >  drivers/media/i2c/adv748x/adv748x-core.c  |  54 +++
> >  drivers/media/i2c/adv748x/adv748x-hdmi.c  | 355 ++
> >  drivers/media/i2c/adv748x/adv748x.h   |  53 ++-
> >  6 files changed, 523 insertions(+), 11 deletions(-)
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio

2020-03-13 Thread Hans Verkuil 
On 3/13/20 9:31 AM, Alex Riesen wrote:
> Hi Hans,
> 
> Hans Verkuil, Fri, Mar 13, 2020 09:21:05 +0100:
>> As a general note for this series: it might be better to have two
>> patch series: one for patches 1 and 3-6 (not sure whether 5 can be included
>> or not), and one where the public API changes (i.e. new V4L2 audio controls)
>> are added. The first can probably be merged fairly quickly, the second will
>> likely require more iterations since public API patches always take much 
>> longer
>> before they are mature.
> 
> I see. After the discussion started, I started to have suspicions of my own
> regarding the V4L2 ioctls. Except for log-status, which is a practical
> diagnostics feature (even supported by v4l2-ctl), I'm thinking about dropping
> them altogether in favor of audio soc DAI implementation.
> The DAI implementation does all we ever needed from the device. Besides,
> selecting a I2S protocol variant from user space (I2S vs I2S/TDM) never felt
> right.

That sounds like what everyone else does as well.

> 
> Shall I submit the log-status separately?

Yes please. In my experience, log status is a very nice and very useful feature.

If you have other sensible cleanups, then feel free to add those as well.

Regards,

Hans

> 
> Regards,
> Alex
> 
>> On 1/13/20 3:14 PM, Alex Riesen wrote:
>>> This adds minimal support for controlling the audio output I2S port 
>>> available
>>> on ADV7481 and ADV7482 HDMI decoder devices by ADI. The port carries audio
>>> signal from the decoded HDMI stream.
>>>
>>> An ADV7482 on the Renesas Salvator-X ES1.1 was used during development of 
>>> this
>>> code.
>>>
>>> Alex Riesen (8):
>>>  1. media: adv748x: add a device-specific wrapper for register block read
>>>  2. media: adv748x: add audio mute control and output selection ioctls
>>>  3. media: adv748x: add log_status ioctl
>>>  4. media: adv748x: reserve space for the audio (I2S) port in the driver
>>> structures
>>>  5. media: adv748x: add an ASoC DAI definition to the driver
>>>  6. media: adv748x: reduce amount of code for bitwise modification of
>>> device registers
>>>  7. dt-bindings: adv748x: add information about serial audio interface
>>> (I2S/TDM)
>>>  8. arm64: dts: renesas: salvator: add a connection from adv748x codec
>>> (HDMI input) to the R-Car SoC
>>>
>>>  .../devicetree/bindings/media/i2c/adv748x.txt |  13 +-
>>>  .../dts/renesas/r8a7795-es1-salvator-x.dts|  24 +-
>>>  .../boot/dts/renesas/salvator-common.dtsi |  35 +-
>>>  drivers/media/i2c/adv748x/adv748x-core.c  |  54 +++
>>>  drivers/media/i2c/adv748x/adv748x-hdmi.c  | 355 ++
>>>  drivers/media/i2c/adv748x/adv748x.h   |  53 ++-
>>>  6 files changed, 523 insertions(+), 11 deletions(-)
>>>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/8] media: i2c: adv748x: add support for HDMI audio

2020-03-13 Thread Alex Riesen 
Hans Verkuil, Fri, Mar 13, 2020 09:37:18 +0100:
> On 3/13/20 9:31 AM, Alex Riesen wrote:
> > Shall I submit the log-status separately?
> 
> Yes please. In my experience, log status is a very nice and very useful 
> feature.
> 
> If you have other sensible cleanups, then feel free to add those as well.

Noted. I shall send it after the DAI series: less inter-series dependencies
this way (the log-status needs a new routine used by the DAI code).

Regards,
Alex
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls

2020-03-13 Thread Alex Riesen 
Hi Hans,

Hans Verkuil, Fri, Mar 13, 2020 09:16:11 +0100:
> On 1/13/20 3:15 PM, Alex Riesen wrote:
> > This change implements audio-related V4L2 ioctls for the HDMI subdevice.
> 
> This is really where things go wrong. These V4L2 audio ioctls are meant for
> old PCI TV tuner devices where the audio was implemented as audio jack outputs
> that are typically looped back to audio inputs on a (PCI) soundcard. And when
> these ioctls were designed ALSA didn't even exist.

I see. That was before my time :)

> Generally an hdmi driver will configure the i2s audio automatically, which is
> typically connected to the SoC and controlled by the ALSA driver of the SoC,
> but there may well be missing features (audio never got a lot of attention in
> hdmi receivers). So what I would like to know is: what features are missing?

Well, the audio is missing. The current adv748x driver does not export the
audio features of the device at all. There is no code to enable the I2S audio
output and it is disabled (all clock and the data lines) by default.

But, by now it seems to be clear that implementation of ALSA SoC DAI
interfaces is the way to support the audio.

And I am already slowly working on it.

> Anything missing can likely be resolved by adding HDMI audio specific V4L2 
> controls,
> which would be the right approach for this.
> 
> So I would expect to see a proposal for V4L2_CID_DV_RX_AUDIO_ controls to be
> added here:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-dv.html

This seems to be an explicitly "digital video" control class. And it has no
control option for mute. Or did you mean a similarly structured new class for
"digital audio"?

This feels like an overkill for this particular driver...

Regards,
Alex

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Outreachy kernel] [PATCH v2] Staging: rtl8723bs: rtw_mlme: Remove unnecessary conditions

2020-03-13 Thread Shreeya Patel 
Remove unnecessary if and else conditions since both are leading to the
initialization of "phtpriv->ampdu_enable" with the same value.
Also, remove the unnecessary else-if condition since it does nothing.

Signed-off-by: Shreeya Patel 
---

Changes in v2
  - Remove unnecessary comments
  - Remove unnecessary else-if condition which does nothing.

 drivers/staging/rtl8723bs/core/rtw_mlme.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 71fcb466019a..d7a58af76ea0 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -2772,16 +2772,7 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 
*pie, uint ie_len, u8 channe
 
/* maybe needs check if ap supports rx ampdu. */
if (!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == 1) {
-   if (pregistrypriv->wifi_spec == 1) {
-   /* remove this part because testbed AP should disable 
RX AMPDU */
-   /* phtpriv->ampdu_enable = false; */
-   phtpriv->ampdu_enable = true;
-   } else {
-   phtpriv->ampdu_enable = true;
-   }
-   } else if (pregistrypriv->ampdu_enable == 2) {
-   /* remove this part because testbed AP should disable RX AMPDU 
*/
-   /* phtpriv->ampdu_enable = true; */
+   phtpriv->ampdu_enable = true;
}
 
/* check Max Rx A-MPDU Size */
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Outreachy kernel] [PATCH v2] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Shreeya Patel 
Remove if and else conditions since both are leading to the
initialization of "valueDMATimeout" and "valueDMAPageCount" with
the same value.

Found using coccinelle script.

Signed-off-by: Shreeya Patel 
---

Changes in v2
  - Remove unnecessary comments.

 drivers/staging/rtl8723bs/hal/sdio_halinit.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c 
b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
index e813382e78a6..4894f7d9a1d4 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
@@ -551,18 +551,8 @@ static void HalRxAggr8723BSdio(struct adapter *padapter)
 
pregistrypriv = &padapter->registrypriv;
 
-   if (pregistrypriv->wifi_spec) {
-   /*  2010.04.27 hpfan */
-   /*  Adjust RxAggrTimeout to close to zero disable RxAggr, 
suggested by designer */
-   /*  Timeout value is calculated by 34 / (2^n) */
-   valueDMATimeout = 0x06;
-   valueDMAPageCount = 0x06;
-   } else {
-   /*  20130530, Isaac@SD1 suggest 3 kinds of parameter */
-   /*  TX/RX Balance */
-   valueDMATimeout = 0x06;
-   valueDMAPageCount = 0x06;
-   }
+   valueDMATimeout = 0x06;
+   valueDMAPageCount = 0x06;
 
rtw_write8(padapter, REG_RXDMA_AGG_PG_TH + 1, valueDMATimeout);
rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, valueDMAPageCount);
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Outreachy kernel] [PATCH v2] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Shreeya Patel 
Remove if and else conditions since both are leading to the
initialization of "valueDMATimeout" and "valueDMAPageCount" with
the same value.

Found using coccinelle script.

Signed-off-by: Shreeya Patel 
---

Changes in v2
  - Remove unnecessary comments.

 drivers/staging/rtl8723bs/hal/sdio_halinit.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/sdio_halinit.c 
b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
index e813382e78a6..4894f7d9a1d4 100644
--- a/drivers/staging/rtl8723bs/hal/sdio_halinit.c
+++ b/drivers/staging/rtl8723bs/hal/sdio_halinit.c
@@ -551,18 +551,8 @@ static void HalRxAggr8723BSdio(struct adapter *padapter)
 
pregistrypriv = &padapter->registrypriv;
 
-   if (pregistrypriv->wifi_spec) {
-   /*  2010.04.27 hpfan */
-   /*  Adjust RxAggrTimeout to close to zero disable RxAggr, 
suggested by designer */
-   /*  Timeout value is calculated by 34 / (2^n) */
-   valueDMATimeout = 0x06;
-   valueDMAPageCount = 0x06;
-   } else {
-   /*  20130530, Isaac@SD1 suggest 3 kinds of parameter */
-   /*  TX/RX Balance */
-   valueDMATimeout = 0x06;
-   valueDMAPageCount = 0x06;
-   }
+   valueDMATimeout = 0x06;
+   valueDMAPageCount = 0x06;
 
rtw_write8(padapter, REG_RXDMA_AGG_PG_TH + 1, valueDMATimeout);
rtw_write8(padapter, REG_RXDMA_AGG_PG_TH, valueDMAPageCount);
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls

2020-03-13 Thread Hans Verkuil 
On 3/13/20 11:26 AM, Alex Riesen wrote:
> Hi Hans,
> 
> Hans Verkuil, Fri, Mar 13, 2020 09:16:11 +0100:
>> On 1/13/20 3:15 PM, Alex Riesen wrote:
>>> This change implements audio-related V4L2 ioctls for the HDMI subdevice.
>>
>> This is really where things go wrong. These V4L2 audio ioctls are meant for
>> old PCI TV tuner devices where the audio was implemented as audio jack 
>> outputs
>> that are typically looped back to audio inputs on a (PCI) soundcard. And when
>> these ioctls were designed ALSA didn't even exist.
> 
> I see. That was before my time :)
> 
>> Generally an hdmi driver will configure the i2s audio automatically, which is
>> typically connected to the SoC and controlled by the ALSA driver of the SoC,
>> but there may well be missing features (audio never got a lot of attention in
>> hdmi receivers). So what I would like to know is: what features are missing?
> 
> Well, the audio is missing. The current adv748x driver does not export the
> audio features of the device at all. There is no code to enable the I2S audio
> output and it is disabled (all clock and the data lines) by default.

Sorry, I was vague in my question. Obviously that needs to be added, but besides
adding the low-level i2s support I was wondering if there are additional things
that need to be exposed to userspace in order for audio to fully work.

> 
> But, by now it seems to be clear that implementation of ALSA SoC DAI
> interfaces is the way to support the audio.
> 
> And I am already slowly working on it.
> 
>> Anything missing can likely be resolved by adding HDMI audio specific V4L2 
>> controls,
>> which would be the right approach for this.
>>
>> So I would expect to see a proposal for V4L2_CID_DV_RX_AUDIO_ controls to be
>> added here:
>>
>> https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-dv.html
> 
> This seems to be an explicitly "digital video" control class. And it has no
> control option for mute. Or did you mean a similarly structured new class for
> "digital audio"?

There are no DV_ audio controls at all today. So any new audio controls would be
added to the DV class. But if there is nothing that needs to be exposed, then
nothing needs to be added :-)

Regards,

Hans

> 
> This feels like an overkill for this particular driver...
> 
> Regards,
> Alex
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH v2] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Dan Carpenter 
Thanks!

You sent the same patch twice.  Next time put a note under the ---
"Resending because I had a typo in Greg's email address".

Reviewed-by: Dan Carpenter 

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Outreachy kernel] [PATCH v2] Staging: rtl8723bs: sdio_halinit: Remove unnecessary conditions

2020-03-13 Thread Dan Carpenter 
Ignore this one.  There was a typo in Greg's email address.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/8] media: adv748x: add audio mute control and output selection ioctls

2020-03-13 Thread Alex Riesen 
Hans Verkuil, Fri, Mar 13, 2020 11:52:03 +0100:
> On 3/13/20 11:26 AM, Alex Riesen wrote:
> > Hans Verkuil, Fri, Mar 13, 2020 09:16:11 +0100:
> >> Generally an hdmi driver will configure the i2s audio automatically, which 
> >> is
> >> typically connected to the SoC and controlled by the ALSA driver of the 
> >> SoC,
> >> but there may well be missing features (audio never got a lot of attention 
> >> in
> >> hdmi receivers). So what I would like to know is: what features are 
> >> missing?
> > 
> > Well, the audio is missing. The current adv748x driver does not export the
> > audio features of the device at all. There is no code to enable the I2S 
> > audio
> > output and it is disabled (all clock and the data lines) by default.
> 
> Sorry, I was vague in my question. Obviously that needs to be added, but 
> besides
> adding the low-level i2s support I was wondering if there are additional 
> things
> that need to be exposed to userspace in order for audio to fully work.

None that I can't expose over the DAI interfaces: clocks, I2S format,
mute/demute ... All covered.

> >> Anything missing can likely be resolved by adding HDMI audio specific V4L2 
> >> controls,
> >> which would be the right approach for this.
> >>
> >> So I would expect to see a proposal for V4L2_CID_DV_RX_AUDIO_ controls to 
> >> be
> >> added here:
> >>
> >> https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-dv.html
> > 
> > This seems to be an explicitly "digital video" control class. And it has no
> > control option for mute. Or did you mean a similarly structured new class 
> > for
> > "digital audio"?
> 
> There are no DV_ audio controls at all today. So any new audio controls would 
> be
> added to the DV class. But if there is nothing that needs to be exposed, then
> nothing needs to be added :-)

Ah, alright. I shall keep an eye on it, maybe I shall find something to expose 
:)

Regards,
Alex
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Outreachy kernel] [PATCH] Staging: wilc1000: cfg80211: Use kmemdup instead of kmalloc and memcpy

2020-03-13 Thread Shreeya Patel 
Replace calls to kmalloc followed by a memcpy with a direct call to
kmemdup.

The Coccinelle semantic patch used to make this change is as follows:
@@
expression from,to,size,flag;
statement S;
@@

-  to = \(kmalloc\|kzalloc\)(size,flag);
+  to = kmemdup(from,size,flag);
   if (to==NULL || ...) S
-  memcpy(to, from, size);

Signed-off-by: Shreeya Patel 
---
 drivers/staging/wilc1000/cfg80211.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/cfg80211.c 
b/drivers/staging/wilc1000/cfg80211.c
index 54e02807cebf..4bdcbc5fd2fd 100644
--- a/drivers/staging/wilc1000/cfg80211.c
+++ b/drivers/staging/wilc1000/cfg80211.c
@@ -1142,14 +1142,13 @@ static int mgmt_tx(struct wiphy *wiphy,
goto out;
}
 
-   mgmt_tx->buff = kmalloc(len, GFP_KERNEL);
+   mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
if (!mgmt_tx->buff) {
ret = -ENOMEM;
kfree(mgmt_tx);
goto out;
}
 
-   memcpy(mgmt_tx->buff, buf, len);
mgmt_tx->size = len;
 
if (ieee80211_is_probe_resp(mgmt->frame_control)) {
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: bcm2835-camera: Drop unused ignore_errors flag

2020-03-13 Thread Nicolas Saenz Julienne 
On Thu, 2020-03-12 at 22:37 +0100, Stefan Wahren wrote:
> The struct bm2835_mmal_v4l2_ctrl contains an ignore_errors flag which
> was always set to false. So drop the unused flag.
> 
> Signed-off-by: Stefan Wahren 
> ---

Reviewed-by: Nicolas Saenz Julienne 



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: bcm2835-camera: Use designators to init V4L2 controls

2020-03-13 Thread Nicolas Saenz Julienne 
On Thu, 2020-03-12 at 22:37 +0100, Stefan Wahren wrote:
> The initializer lists for the V4L2 controls are hard to read.
> So improve this by using designators.
> 
> Signed-off-by: Stefan Wahren 
> ---

Reviewed-by: Nicolas Saenz Julienne 



signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/5] staging: wfx: make warning about pending frame less scary

2020-03-13 Thread Jérôme Pouiller 
On Thursday 12 March 2020 15:30:19 CET Dan Carpenter wrote:
> On Tue, Mar 10, 2020 at 11:13:54AM +0100, Jerome Pouiller wrote:
[...]
> So it really helps me if the commit message restates the subject.  The
> truth is that I don't really even like the advice that Josh wrote in
> the howto about patch descriptions.  I normally start by explaining the
> problem then how I solved it.  But I try not to be a pedant, so long as
> I can understand the problem and the patch that's fine.  So how I would
> write this commit message is:
> 
> The warning message about releasing a station while Tx is in
> progress will trigger a stack trace, possibly a reboot depending
> on the configuration, and a syzbot email.  It's not necessarily
> a big deal that transmission is still in process so let's make the
> warning less scary.

Indeed, my idea was the reviewers start by reading subjects and then read
the body of the commit. I will care now.


> > Signed-off-by: Jérôme Pouiller 
> > ---
> >  drivers/staging/wfx/sta.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> > index 03d0f224ffdb..010e13bcd33e 100644
> > --- a/drivers/staging/wfx/sta.c
> > +++ b/drivers/staging/wfx/sta.c
> > @@ -605,7 +605,9 @@ int wfx_sta_remove(struct ieee80211_hw *hw, struct 
> > ieee80211_vif *vif,
> >   int i;
> >
> >   for (i = 0; i < ARRAY_SIZE(sta_priv->buffered); i++)
> > - WARN(sta_priv->buffered[i], "release station while Tx is in 
> > progress");
> > + if (sta_priv->buffered[i])
> > + dev_warn(wvif->wdev->dev, "release station while %d 
> > pending frame on queue %d",
> > +  sta_priv->buffered[i], i);
> 
> Why print a warning message at all if this is a normal situation?  Just
> delete the whole thing.

I saw cases where it happened and it seems harmless. In add, this code
is going to be released with 5.6. So, the WARN have to be removed.

However, I think it is not normal. Even if it is harmless, it is the
symptom of something unclean.

So, I think that dev_warn() is the correct level of notification.

(I should have included that in the commit log)

-- 
Jérôme Pouiller

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 3/6] staging: mt7621-dts: make use of 'reset-gpios' property for pci

2020-03-13 Thread Sergio Paracuellos 
Properly set pins for group pcie as 'gpio' function and declare
gpio's in the pci node to make reset stuff properly functional.
Delete no more needed general reset and previous pers gpio which
is now being used in 'reset-gpios' property.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi 
b/drivers/staging/mt7621-dts/mt7621.dtsi
index d89d68ffa7bc..488474153535 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -286,7 +286,7 @@ mdio0 {
pcie_pins: pcie0 {
pcie0 {
groups = "pcie";
-   function = "pcie rst";
+   function = "gpio";
};
};
 
@@ -512,7 +512,6 @@ pcie: pcie@1e14 {
#address-cells = <3>;
#size-cells = <2>;
 
-   perst-gpio = <&gpio 19 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&pcie_pins>;
 
@@ -532,13 +531,17 @@ pcie: pcie@1e14 {
 
status = "disabled";
 
-   resets = <&rstctrl 23 &rstctrl 24 &rstctrl 25 &rstctrl 26>;
-   reset-names = "pcie", "pcie0", "pcie1", "pcie2";
+   resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
+   reset-names = "pcie0", "pcie1", "pcie2";
clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
clock-names = "pcie0", "pcie1", "pcie2";
phys = <&pcie0_phy 0>, <&pcie0_phy 1>, <&pcie1_phy 0>;
phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
 
+   reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
+   <&gpio 8 GPIO_ACTIVE_LOW>,
+   <&gpio 7 GPIO_ACTIVE_LOW>;
+
pcie@0,0 {
reg = <0x 0 0 0 0>;
#address-cells = <3>;
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 0/6] staging: mt7621-pci: re-do reset boot process

2020-03-13 Thread Sergio Paracuellos 
Some time ago Greg Ungerer reported some random hangs using
the staging mt7621-pci driver:

See:
* 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2019-June/134947.html
 

Try to fix that is the main motivation of this patch series.

Also in openwrt there is a driver for mt7621-pci which seems was rewritten
from scratch (for kernel 4.14) by Ryder Lee and Weijie Gao from mediatek. 
There the approach for reset assert-deassert process is to set as 'gpio'
the function for all the 'pcie' group for the pinctrl driver and use those
gpio's as a reset for the end points. The driver I am talking about is still
using legacy pci and legacy gpio kernel interfaces. IMHO, the correct thing
to do is make this staging driver properly clean and functional and put it
in its correct place in the mainline.

See:
* https://gist.github.com/dengqf6/7a9e9b4032d99f1a91dd9256c8a65c36

Because of all of this this patch series tries to avoid random hangs of boot
trying to use the 'reset-gpios' approach.

Changes are being tested by openwrt people and seems to work.

Hope this helps.

Changes in v4:
* Make use of 'devm_gpiod_get_index_optional' instead of 'devm_gpiod_get_index'.
* Use 'dev_err' instead of 'dev_notice' and return ERR_PTR if 
'devm_gpiod_get_index_optional' fails.
* Rename pers dealy macro to PERST_DELAY_MS.
* Add new patch 6 which removes function 'mt7621_reset_port' not needed anymore.

Changes in v3:
* Avoid to fail if gpio descriptor fails on get.
* re-do PATCH 1 commit message.
* Take into account gpio low polarity on request and assert and deassert.
* Review error path of driver to properly release gpio's resources.

Changes in v2:
* restore configuration for pers mode to GPIO.
* Avoid to read FTS_NUM register in reset state.
* Release gpio's patch added

Best regards,
Sergio Paracuellos


Sergio Paracuellos (6):
  staging: mt7621-pci: use gpios for properly reset
  staging: mt7621-pci: change value for 'PERST_DELAY_MS'
  staging: mt7621-dts: make use of 'reset-gpios' property for pci
  staging: mt7621-pci: bindings: update doc accordly to last changes
  staging: mt7621-pci: release gpios after pci initialization
  staging: mt7621-pci: delete no more needed 'mt7621_reset_port'

 drivers/staging/mt7621-dts/mt7621.dtsi|  11 +-
 .../mt7621-pci/mediatek,mt7621-pci.txt|   7 +-
 drivers/staging/mt7621-pci/pci-mt7621.c   | 122 ++
 3 files changed, 82 insertions(+), 58 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 4/6] staging: mt7621-pci: bindings: update doc accordly to last changes

2020-03-13 Thread Sergio Paracuellos 
Properly update bindings documentation with added 'reset-gpios'
property. Delete also 'perst-gpio' which is not being used anymore.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt 
b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
index 604ec813bd45..327a68267309 100644
--- a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
+++ b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
@@ -6,7 +6,6 @@ Required properties:
 - reg: Base addresses and lengths of the PCIe subsys and root ports.
 - bus-range: Range of bus numbers associated with this controller.
 - #address-cells: Address representation for root ports (must be 3)
-- perst-gpio: PCIe reset signal line.
 - pinctrl-names : The pin control state names.
 - pinctrl-0: The "default" pinctrl state.
 - #size-cells: Size representation for root ports (must be 2)
@@ -24,6 +23,7 @@ Required properties:
   See ../clocks/clock-bindings.txt for details.
 - clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
   root ports.
+- reset-gpios: GPIO specs for the reset pins.
 
 In addition, the device tree node must have sub-nodes describing each PCIe port
 interface, having the following mandatory properties:
@@ -49,7 +49,6 @@ Example for MT7621:
#address-cells = <3>;
#size-cells = <2>;
 
-   perst-gpio = <&gpio 19 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&pcie_pins>;
 
@@ -74,6 +73,10 @@ Example for MT7621:
clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
clock-names = "pcie0", "pcie1", "pcie2";
 
+   reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
+   <&gpio 8 GPIO_ACTIVE_LOW>,
+   <&gpio 7 GPIO_ACTIVE_LOW>;
+
pcie@0,0 {
reg = <0x 0 0 0 0>;
#address-cells = <3>;
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 6/6] staging: mt7621-pci: delete no more needed 'mt7621_reset_port'

2020-03-13 Thread Sergio Paracuellos 
After review all the resets at the beggining the function
'mt7621_reset_port' is not needed anymore. Hence delete it
and its uses.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 0d9dd14f6bec..973be9aa7bb2 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -255,13 +255,6 @@ static inline void mt7621_control_deassert(struct 
mt7621_pcie_port *port)
reset_control_assert(port->pcie_rst);
 }
 
-static void mt7621_reset_port(struct mt7621_pcie_port *port)
-{
-   mt7621_control_assert(port);
-   msleep(100);
-   mt7621_control_deassert(port);
-}
-
 static void setup_cm_memory_region(struct mt7621_pcie *pcie)
 {
struct resource *mem_resource = &pcie->mem;
@@ -427,12 +420,6 @@ static int mt7621_pcie_init_port(struct mt7621_pcie_port 
*port)
u32 slot = port->slot;
int err;
 
-   /*
-* Any MT7621 Ralink pcie controller that doesn't have 0x0101 at
-* the end of the chip_id has inverted PCI resets.
-*/
-   mt7621_reset_port(port);
-
err = phy_init(port->phy);
if (err) {
dev_err(dev, "failed to initialize port%d phy\n", slot);
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 5/6] staging: mt7621-pci: release gpios after pci initialization

2020-03-13 Thread Sergio Paracuellos 
R3G's LEDs fail to initialize because one of them uses GPIO8
Hence, release the GPIO resources after PCIe initialization
and properly release also in driver error path.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 3d85ce788f9f..0d9dd14f6bec 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -484,6 +484,15 @@ static void mt7621_pcie_reset_ep_deassert(struct 
mt7621_pcie *pcie)
mdelay(PERST_DELAY_MS);
 }
 
+static void mt7621_pcie_release_gpios(struct mt7621_pcie *pcie)
+{
+   struct mt7621_pcie_port *port;
+
+   list_for_each_entry(port, &pcie->ports, list)
+   if (port->gpio_rst)
+   gpiod_put(port->gpio_rst);
+}
+
 static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
 {
struct device *dev = pcie->dev;
@@ -683,7 +692,8 @@ static int mt7621_pci_probe(struct platform_device *pdev)
err = mt7621_pcie_init_virtual_bridges(pcie);
if (err) {
dev_err(dev, "Nothing is connected in virtual bridges. 
Exiting...");
-   return 0;
+   err = 0;
+   goto out_release_gpios;
}
 
mt7621_pcie_enable_ports(pcie);
@@ -691,7 +701,7 @@ static int mt7621_pci_probe(struct platform_device *pdev)
err = mt7621_pci_parse_request_of_pci_ranges(pcie);
if (err) {
dev_err(dev, "Error requesting pci resources from ranges");
-   return err;
+   goto out_release_gpios;
}
 
setup_cm_memory_region(pcie);
@@ -699,16 +709,19 @@ static int mt7621_pci_probe(struct platform_device *pdev)
err = mt7621_pcie_request_resources(pcie, &res);
if (err) {
dev_err(dev, "Error requesting resources\n");
-   return err;
+   goto out_release_gpios;
}
 
err = mt7621_pcie_register_host(bridge, &res);
if (err) {
dev_err(dev, "Error registering host\n");
-   return err;
+   goto out_release_gpios;
}
 
-   return 0;
+out_release_gpios:
+   mt7621_pcie_release_gpios(pcie);
+
+   return err;
 }
 
 static const struct of_device_id mt7621_pci_ids[] = {
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/6] staging: mt7621-pci: change value for 'PERST_DELAY_MS'

2020-03-13 Thread Sergio Paracuellos 
Value of 'PERST_DELAY_MS' is too high and it is ok just
to set up to 100 ms. Update also define name from
'PERST_DELAY_US' into 'PERST_DELAY_MS'

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 9b4fe8d31101..3d85ce788f9f 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -86,7 +86,7 @@
 #define MEMORY_BASE0x0
 #define PERST_MODE_MASKGENMASK(11, 10)
 #define PERST_MODE_GPIOBIT(10)
-#define PERST_DELAY_US 1000
+#define PERST_DELAY_MS 100
 
 /**
  * struct mt7621_pcie_port - PCIe port information
@@ -463,7 +463,7 @@ static void mt7621_pcie_reset_assert(struct mt7621_pcie 
*pcie)
mt7621_rst_gpio_pcie_assert(port);
}
 
-   mdelay(PERST_DELAY_US);
+   mdelay(PERST_DELAY_MS);
 }
 
 static void mt7621_pcie_reset_rc_deassert(struct mt7621_pcie *pcie)
@@ -481,7 +481,7 @@ static void mt7621_pcie_reset_ep_deassert(struct 
mt7621_pcie *pcie)
list_for_each_entry(port, &pcie->ports, list)
mt7621_rst_gpio_pcie_deassert(port);
 
-   mdelay(PERST_DELAY_US);
+   mdelay(PERST_DELAY_MS);
 }
 
 static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/6] staging: mt7621-pci: use gpios for properly reset

2020-03-13 Thread Sergio Paracuellos 
Original driver code was using three gpio's for reset
asserts and deasserts the pcis. Instead of using that
a general reset control with a perst gpio was introduced
and it seems it is partially working but sometimes there
are some unexpected hangs on boot. This commit make use of
the three original gpios using 'reset-gpios' property of
the device tree and removes the reset line and perst gpio.
According to the mediatek aplication note v0.1 there are
three gpios used for pcie ports reset control: gpio#19,
gpio#8 and gpio#7 for slots 0, 1 and 2 respectively.
This schema can be used separately for mt7621A but in some
boards due to pin share issue, if the PCM and I2S function
are enable at the same time, there are no enough GPIO to
control per-port PCIe reset. In those cases gpio#19 is enought
for reset the three ports together. Because of this we just
try to get the three gpios but if some of them fail we are not
failing in boot process, just prints a kernel notice and take
after into account if the descriptor is or not valid in order
to use it. All of them are set as GPIO output low configuration.
The gpio descriptor's API takes device tree property into account
and invert value if the pin is configured as active low.
So we also have to properly request pins from device tree
and set values correct in assert and deassert functions.
After this changes the order to make all assert and
deassert in the 'probe' process makes more sense:
* Parse device tree.
* make assert of the RC's and EP's before doing anything else.
* make deassert of the RC's before initializing the phy.
* Init the phy.
* make deassert of the EP's before initialize pci ports.
* Normal PCI initialization.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 84 +++--
 1 file changed, 51 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c 
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 770dabd3a70d..9b4fe8d31101 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -95,6 +95,7 @@
  * @pcie: pointer to PCIe host info
  * @phy: pointer to PHY control block
  * @pcie_rst: pointer to port reset control
+ * @gpio_rst: gpio reset
  * @slot: port slot
  * @enabled: indicates if port is enabled
  */
@@ -104,6 +105,7 @@ struct mt7621_pcie_port {
struct mt7621_pcie *pcie;
struct phy *phy;
struct reset_control *pcie_rst;
+   struct gpio_desc *gpio_rst;
u32 slot;
bool enabled;
 };
@@ -117,8 +119,6 @@ struct mt7621_pcie_port {
  * @offset: IO / Memory offset
  * @dev: Pointer to PCIe device
  * @ports: pointer to PCIe port information
- * @perst: gpio reset
- * @rst: pointer to pcie reset
  * @resets_inverted: depends on chip revision
  * reset lines are inverted.
  */
@@ -133,8 +133,6 @@ struct mt7621_pcie {
resource_size_t io;
} offset;
struct list_head ports;
-   struct gpio_desc *perst;
-   struct reset_control *rst;
bool resets_inverted;
 };
 
@@ -210,16 +208,16 @@ static void write_config(struct mt7621_pcie *pcie, 
unsigned int dev,
pcie_write(pcie, val, RALINK_PCI_CONFIG_DATA);
 }
 
-static inline void mt7621_perst_gpio_pcie_assert(struct mt7621_pcie *pcie)
+static inline void mt7621_rst_gpio_pcie_assert(struct mt7621_pcie_port *port)
 {
-   gpiod_set_value(pcie->perst, 0);
-   mdelay(PERST_DELAY_US);
+   if (port->gpio_rst)
+   gpiod_set_value(port->gpio_rst, 1);
 }
 
-static inline void mt7621_perst_gpio_pcie_deassert(struct mt7621_pcie *pcie)
+static inline void mt7621_rst_gpio_pcie_deassert(struct mt7621_pcie_port *port)
 {
-   gpiod_set_value(pcie->perst, 1);
-   mdelay(PERST_DELAY_US);
+   if (port->gpio_rst)
+   gpiod_set_value(port->gpio_rst, 0);
 }
 
 static inline bool mt7621_pcie_port_is_linkup(struct mt7621_pcie_port *port)
@@ -367,6 +365,13 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
if (IS_ERR(port->phy))
return PTR_ERR(port->phy);
 
+   port->gpio_rst = devm_gpiod_get_index_optional(dev, "reset", slot,
+  GPIOD_OUT_LOW);
+   if (IS_ERR(port->gpio_rst)) {
+   dev_err(dev, "Failed to get GPIO for PCIe%d\n", slot);
+   return PTR_ERR(port->gpio_rst);
+   }
+
port->slot = slot;
port->pcie = pcie;
 
@@ -383,12 +388,6 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
struct resource regs;
int err;
 
-   pcie->perst = devm_gpiod_get(dev, "perst", GPIOD_OUT_HIGH);
-   if (IS_ERR(pcie->perst)) {
-   dev_err(dev, "failed to get gpio perst\n");
-   return PTR_ERR(pcie->perst);
-   }
-
err = of_address_to_resource(node, 0, ®s);
if (err) {
dev_err(dev, "missing \"reg\" property\n");
@@ -399,12 +398,6 @@ static int mt7621_pcie_parse_dt(str

Re: [Outreachy kernel] [PATCH v2] Staging: rtl8723bs: rtw_mlme: Remove unnecessary conditions

2020-03-13 Thread Joe Perches 
On Fri, 2020-03-13 at 15:59 +0530, Shreeya Patel wrote:
> Remove unnecessary if and else conditions since both are leading to the
> initialization of "phtpriv->ampdu_enable" with the same value.
> Also, remove the unnecessary else-if condition since it does nothing.
[]
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c 
> b/drivers/staging/rtl8723bs/core/rtw_mlme.c
[]
> @@ -2772,16 +2772,7 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 
> *pie, uint ie_len, u8 channe
>  
>   /* maybe needs check if ap supports rx ampdu. */
>   if (!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == 1) {
> - if (pregistrypriv->wifi_spec == 1) {
> - /* remove this part because testbed AP should disable 
> RX AMPDU */
> - /* phtpriv->ampdu_enable = false; */
> - phtpriv->ampdu_enable = true;
> - } else {
> - phtpriv->ampdu_enable = true;
> - }
> - } else if (pregistrypriv->ampdu_enable == 2) {
> - /* remove this part because testbed AP should disable RX AMPDU 
> */
> - /* phtpriv->ampdu_enable = true; */
> + phtpriv->ampdu_enable = true;

This isn't the same test.

This could be:
if ((!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == 1)) ||
pregistrypriv->ampdu_enable == 2)
phtpriv->ampdu_enable = true;

Though it is probably more sensible to just set
phtpriv->ampdu_enable without testing whether or
not it's already set:

if (pregistrypriv->ampdu_enable == 1 ||
pregistrypriv->ampdu_enable == 2)
phtpriv->ampdu_enable = true;


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel