[PATCH v3] ARM: dts: imx6qdl-icore: Fix OTG_ID pin and sdcard detect

2020-07-17 Thread Suniel Mahesh 
From: Michael Trimarchi 

The current pin muxing scheme muxes GPIO_1 pad for USB_OTG_ID
because of which when card is inserted, usb otg is enumerated
and the card is never detected.

[   64.492645] cfg80211: failed to load regulatory.db
[   64.492657] imx-sdma 20ec000.sdma: external firmware not found, using ROM 
firmware
[   76.343711] ci_hdrc ci_hdrc.0: EHCI Host Controller
[   76.349742] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 2
[   76.388862] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
[   76.396650] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002, 
bcdDevice= 5.08
[   76.405412] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[   76.412763] usb usb2: Product: EHCI Host Controller
[   76.417666] usb usb2: Manufacturer: Linux 5.8.0-rc1-next-20200618 ehci_hcd
[   76.424623] usb usb2: SerialNumber: ci_hdrc.0
[   76.431755] hub 2-0:1.0: USB hub found
[   76.435862] hub 2-0:1.0: 1 port detected

The TRM mentions GPIO_1 pad should be muxed/assigned for card detect
and ENET_RX_ER pad for USB_OTG_ID for proper operation.

This patch fixes pin muxing as per TRM and is tested on a
i.Core 1.5 MX6 DL SOM.

[   22.449165] mmc0: host does not support reading read-only switch, assuming 
write-enable
[   22.459992] mmc0: new high speed SDHC card at address 0001
[   22.469725] mmcblk0: mmc0:0001 EB1QT 29.8 GiB
[   22.478856]  mmcblk0: p1 p2

Fixes: 6df11287f7c9 ("ARM: dts: imx6q: Add Engicam i.CoreM6 Quad/Dual initial 
support")
Cc: sta...@vger.kernel.org
Signed-off-by: Michael Trimarchi 
Signed-off-by: Suniel Mahesh 
---
Changes for v3:
- Changed subject of the patch, added fixes tag and copied stable kernel
  as suggested by Shawn Guo.

Changes for v2:
- Changed patch description as suggested by Michael Trimarchi to make it
  more readable/understandable.

NOTE:
- patch tested on i.Core 1.5 MX6 DL
---
 arch/arm/boot/dts/imx6qdl-icore.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-icore.dtsi 
b/arch/arm/boot/dts/imx6qdl-icore.dtsi
index f2f475e..23c318d 100644
--- a/arch/arm/boot/dts/imx6qdl-icore.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-icore.dtsi
@@ -398,7 +398,7 @@
 
pinctrl_usbotg: usbotggrp {
fsl,pins = <
-   MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
+   MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x17059
>;
};
 
@@ -410,6 +410,7 @@
MX6QDL_PAD_SD1_DAT1__SD1_DATA1 0x17070
MX6QDL_PAD_SD1_DAT2__SD1_DATA2 0x17070
MX6QDL_PAD_SD1_DAT3__SD1_DATA3 0x17070
+   MX6QDL_PAD_GPIO_1__GPIO1_IO01  0x1b0b0
>;
};
 
-- 
2.7.4

[PATCH v2] arch: arm: imx6qdl-icore: Fix OTG_ID pin and sdcard detect

2020-06-19 Thread Suniel Mahesh 
From: Michael Trimarchi 

The current pin muxing scheme muxes GPIO_1 pad for USB_OTG_ID
because of which when card is inserted, usb otg is enumerated
and the card is never detected.

[   64.492645] cfg80211: failed to load regulatory.db
[   64.492657] imx-sdma 20ec000.sdma: external firmware not found, using ROM 
firmware
[   76.343711] ci_hdrc ci_hdrc.0: EHCI Host Controller
[   76.349742] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 2
[   76.388862] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
[   76.396650] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002, 
bcdDevice= 5.08
[   76.405412] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[   76.412763] usb usb2: Product: EHCI Host Controller
[   76.417666] usb usb2: Manufacturer: Linux 5.8.0-rc1-next-20200618 ehci_hcd
[   76.424623] usb usb2: SerialNumber: ci_hdrc.0
[   76.431755] hub 2-0:1.0: USB hub found
[   76.435862] hub 2-0:1.0: 1 port detected

The TRM mentions GPIO_1 pad should be muxed/assigned for card detect
and ENET_RX_ER pad for USB_OTG_ID for proper operation.

This patch fixes pin muxing as per TRM and is tested on a
i.Core 1.5 MX6 DL SOM.

[   22.449165] mmc0: host does not support reading read-only switch, assuming 
write-enable
[   22.459992] mmc0: new high speed SDHC card at address 0001
[   22.469725] mmcblk0: mmc0:0001 EB1QT 29.8 GiB
[   22.478856]  mmcblk0: p1 p2

Signed-off-by: Michael Trimarchi 
Signed-off-by: Suniel Mahesh 
---
Changes for v2:
- Changed patch description as suggested by Michael Trimarchi to make it
  more readable/understandable.
---
 arch/arm/boot/dts/imx6qdl-icore.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-icore.dtsi 
b/arch/arm/boot/dts/imx6qdl-icore.dtsi
index 756f3a9..12997da 100644
--- a/arch/arm/boot/dts/imx6qdl-icore.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-icore.dtsi
@@ -397,7 +397,7 @@
 
pinctrl_usbotg: usbotggrp {
fsl,pins = <
-   MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
+   MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x17059
>;
};
 
@@ -409,6 +409,7 @@
MX6QDL_PAD_SD1_DAT1__SD1_DATA1 0x17070
MX6QDL_PAD_SD1_DAT2__SD1_DATA2 0x17070
MX6QDL_PAD_SD1_DAT3__SD1_DATA3 0x17070
+   MX6QDL_PAD_GPIO_1__GPIO1_IO01  0x1b0b0
>;
};
 
-- 
2.7.4

[PATCH] arch: arm: imx6qdl-icore: Fix OTG_ID pin and sdcard detect

2020-06-19 Thread Suniel Mahesh 
From: Michael Trimarchi 

The current pin muxing scheme muxes GPIO_1 pad for USB_OTG_ID
but the TRM mentions GPIO_1 pad is muxed for card detetcion,
because of which when card is inserted, usb otg is enumerated
and the card is never detected.

[   64.492645] cfg80211: failed to load regulatory.db
[   64.492657] imx-sdma 20ec000.sdma: external firmware not found, using ROM 
firmware
[   76.343711] ci_hdrc ci_hdrc.0: EHCI Host Controller
[   76.349742] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 2
[   76.388862] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
[   76.396650] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002, 
bcdDevice= 5.08
[   76.405412] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[   76.412763] usb usb2: Product: EHCI Host Controller
[   76.417666] usb usb2: Manufacturer: Linux 5.8.0-rc1-next-20200618 ehci_hcd
[   76.424623] usb usb2: SerialNumber: ci_hdrc.0
[   76.431755] hub 2-0:1.0: USB hub found
[   76.435862] hub 2-0:1.0: 1 port detected

Fix the pin muxing as per TRM by muxing ENET_RX_ER pad for USB_OTG_ID
and GPIO_1 pad for card detect.

[   22.449165] mmc0: host does not support reading read-only switch, assuming 
write-enable
[   22.459992] mmc0: new high speed SDHC card at address 0001
[   22.469725] mmcblk0: mmc0:0001 EB1QT 29.8 GiB
[   22.478856]  mmcblk0: p1 p2

Cc: sta...@vger.kernel.org
Signed-off-by: Michael Trimarchi 
Signed-off-by: Suniel Mahesh 
---
NOTE:
- patch tested on i.Core 1.5 MX6 DL
---
 arch/arm/boot/dts/imx6qdl-icore.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-icore.dtsi 
b/arch/arm/boot/dts/imx6qdl-icore.dtsi
index 756f3a9..12997da 100644
--- a/arch/arm/boot/dts/imx6qdl-icore.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-icore.dtsi
@@ -397,7 +397,7 @@
 
pinctrl_usbotg: usbotggrp {
fsl,pins = <
-   MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
+   MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x17059
>;
};
 
@@ -409,6 +409,7 @@
MX6QDL_PAD_SD1_DAT1__SD1_DATA1 0x17070
MX6QDL_PAD_SD1_DAT2__SD1_DATA2 0x17070
MX6QDL_PAD_SD1_DAT3__SD1_DATA3 0x17070
+   MX6QDL_PAD_GPIO_1__GPIO1_IO01  0x1b0b0
>;
};
 
-- 
2.7.4

Machine specific static mappings iotable_init(), are they required ?

2019-08-06 Thread Suniel Mahesh 
Hi,

I am trying to port a machine based on arm926 with MMU, having 64MB of RAM.  

I am trying to understand the difference between: 

machine specific static I/O mappings which are done via iotable_init()
(done via callback .map_io in DT_MACHINE_START) and 
dynamic I/O mappings done via ioremap()

In the kernel docs/mailing list, I have encountered a statement which states:

"with machine specific static I/O mappings which are done via iotable_init(), 
registers can be mapped at the upper end of vmalloc area so that one can use as
little of the VA space as possible so vmalloc and friends have a better chance 
of 
getting memory"

I am writing board initialization C file and got stuck at .map_io callback 
function,
whether to define it or not. If yes, under what scenario should I do it

now-a-days I think less boards are using iotable_init(). (is this defunct) ? 
I might be wrong here

Can't I use ioremap and do dynamic mappings when ever required via device tree ?
If I do so will I encounter any problems with vmalloc area.


Thanks & Regards
-- 
Suniel Mahesh

Re: [PATCH v3] staging: ccree: fix boolreturn.cocci warning

2017-10-19 Thread Suniel Mahesh 
On Thursday 19 October 2017 02:24 AM, Tobin C. Harding wrote:
> Hi Suniel,
> 
> Well done with you continued versions. I am being particularly nit picky here 
> but since we are
> striving for perfection I'm sure will humour me. If English is not your first 
> language please
> forgive me for picking you up on language subtleties.

Hi Tobin,

First of all, I thank you very much for the reviews, to be honest I enjoyed the 
process. Yes all of 
us, here we are striving for perfection. I am always open to take suggestions 
from the community to 
improve things which I am working on and there by improving myself. Yeah 
English is not my first
language, but all my education was done in English, no issues there.

> 
> On Wed, Oct 18, 2017 at 12:11:55PM +0530, suni...@techveda.org wrote:
>> From: Suniel Mahesh <suni...@techveda.org>
>>
>> This fixes the following coccinelle warning:
>> WARNING: return of 0/1 in function 'ssi_is_hw_key' with return type bool.
> 
> This should be a description of the problem, so saying _why_ there is a 
> problem or _what_ is wrong
> with the code currently that warrants a patch. Sometimes while describing the 
> problem you may
> include descriptions of the solution especially it is not immediately obvious 
> why your proposed
> solution fixes the issue being explained. As an extra we shouldn't ever say 
> 'This patch ...' or
> 'This does xyz'.
> 
>> return "false" instead of 0.
> 
> Perfect, this is in imperative mood. Spot on!
> 
>> Signed-off-by: Suniel Mahesh <suni...@techveda.org>
>> ---
>> Changes for v3:
>> - Changed the commit log even more to give an accurate
>>   description of the changeset as suggested by Toby C.Harding.
> 
> My name is Tobin :)

how did I blind myself, my bad, will be careful and avoid such mistakes moving 
forward.

> 
>> ---
>> Changes for v2:
>> - Changed the commit log to give a more accurate description
>>   of the changeset as suggested by Toby C.Harding.
>> ---
>> Note:
>> - Patch was built(ARCH=arm) on latest linux-next.
>> - No build issues reported, however it was not
>>   tested on real hardware.
>> - Please discard this changeset, if this is not
>>   helping the code look better.
>> ---
>>  drivers/staging/ccree/ssi_cipher.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_cipher.h 
>> b/drivers/staging/ccree/ssi_cipher.h
>> index c9a83df..f499962 100644
>> --- a/drivers/staging/ccree/ssi_cipher.h
>> +++ b/drivers/staging/ccree/ssi_cipher.h
>> @@ -75,7 +75,7 @@ struct arm_hw_key_info {
>>  
>>  static inline bool ssi_is_hw_key(struct crypto_tfm *tfm)
>>  {
>> -return 0;
>> +return false;
>>  }
>>  
>>  #endif /* CRYPTO_TFM_REQ_HW_KEY */
>> -- 
>> 1.9.1
>>
> 
> For what it's worth, Reviewed-by: Tobin C. Harding <m...@tobin.cc>
> 
> As stated I am being particularly 'nit picky', the commit log is _probably_ 
> good enough to be
> merged, I am not a maintainer though so it's not really anything to do with 
> me. I do know however
> that sometimes patches go to the bottom of Greg's list if they have 
> comments/suggestions. I mention
> this only so you learn more about the process and to help you with 
> successfully getting you patches
> merged. Keep up the work!

Thanks once again Tobin, I love feedback and that's how we can make this world 
a better workplace.

Suniel

> 
> Good luck,
> Tobin.
>

Re: [PATCH v3] staging: ccree: fix boolreturn.cocci warning

2017-10-19 Thread Suniel Mahesh 
On Thursday 19 October 2017 02:24 AM, Tobin C. Harding wrote:
> Hi Suniel,
> 
> Well done with you continued versions. I am being particularly nit picky here 
> but since we are
> striving for perfection I'm sure will humour me. If English is not your first 
> language please
> forgive me for picking you up on language subtleties.

Hi Tobin,

First of all, I thank you very much for the reviews, to be honest I enjoyed the 
process. Yes all of 
us, here we are striving for perfection. I am always open to take suggestions 
from the community to 
improve things which I am working on and there by improving myself. Yeah 
English is not my first
language, but all my education was done in English, no issues there.

> 
> On Wed, Oct 18, 2017 at 12:11:55PM +0530, suni...@techveda.org wrote:
>> From: Suniel Mahesh 
>>
>> This fixes the following coccinelle warning:
>> WARNING: return of 0/1 in function 'ssi_is_hw_key' with return type bool.
> 
> This should be a description of the problem, so saying _why_ there is a 
> problem or _what_ is wrong
> with the code currently that warrants a patch. Sometimes while describing the 
> problem you may
> include descriptions of the solution especially it is not immediately obvious 
> why your proposed
> solution fixes the issue being explained. As an extra we shouldn't ever say 
> 'This patch ...' or
> 'This does xyz'.
> 
>> return "false" instead of 0.
> 
> Perfect, this is in imperative mood. Spot on!
> 
>> Signed-off-by: Suniel Mahesh 
>> ---
>> Changes for v3:
>> - Changed the commit log even more to give an accurate
>>   description of the changeset as suggested by Toby C.Harding.
> 
> My name is Tobin :)

how did I blind myself, my bad, will be careful and avoid such mistakes moving 
forward.

> 
>> ---
>> Changes for v2:
>> - Changed the commit log to give a more accurate description
>>   of the changeset as suggested by Toby C.Harding.
>> ---
>> Note:
>> - Patch was built(ARCH=arm) on latest linux-next.
>> - No build issues reported, however it was not
>>   tested on real hardware.
>> - Please discard this changeset, if this is not
>>   helping the code look better.
>> ---
>>  drivers/staging/ccree/ssi_cipher.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_cipher.h 
>> b/drivers/staging/ccree/ssi_cipher.h
>> index c9a83df..f499962 100644
>> --- a/drivers/staging/ccree/ssi_cipher.h
>> +++ b/drivers/staging/ccree/ssi_cipher.h
>> @@ -75,7 +75,7 @@ struct arm_hw_key_info {
>>  
>>  static inline bool ssi_is_hw_key(struct crypto_tfm *tfm)
>>  {
>> -return 0;
>> +return false;
>>  }
>>  
>>  #endif /* CRYPTO_TFM_REQ_HW_KEY */
>> -- 
>> 1.9.1
>>
> 
> For what it's worth, Reviewed-by: Tobin C. Harding 
> 
> As stated I am being particularly 'nit picky', the commit log is _probably_ 
> good enough to be
> merged, I am not a maintainer though so it's not really anything to do with 
> me. I do know however
> that sometimes patches go to the bottom of Greg's list if they have 
> comments/suggestions. I mention
> this only so you learn more about the process and to help you with 
> successfully getting you patches
> merged. Keep up the work!

Thanks once again Tobin, I love feedback and that's how we can make this world 
a better workplace.

Suniel

> 
> Good luck,
> Tobin.
>

Re: [PATCH] staging: ccree: local variable "dev" not required

2017-10-05 Thread Suniel Mahesh 
On Thursday 05 October 2017 11:30 PM, Joe Perches wrote:
> On Thu, 2017-10-05 at 10:07 +0300, Gilad Ben-Yossef wrote:
>> On Wed, Oct 4, 2017 at 10:12 PM,   wrote:
>>> There is no need to create a local pointer variable "dev" and
>>> pass it various API's, instead use plat_dev which is enumerated
>>> by platform core on successful probe.
> []
>> I'm sorry but I don't understand what is the problem you are trying to solve 
>> or
>> what is the improvement suggested.

Hi Gilad and all,

Actually this patch is not a major improvement nor trying to solve some thing.

As struct device is a member of struct platform_device, I thought why can't we 
use
plat_dev (pointer to struct platform_device) through out the code. May be I was 
trying
to make code look compact, may be I am wrong.

>>
>> Why is having a local variable undesirable? I think having it makes
>> the code more readable, not less.
> 
> IMO: The local variable is _not_ undesirable.
>  It does make the code more readable and
>  shortens line lengths too.
> 

Yes, as you guys pointed out, we can use local variables. It is definitely 
making
the code more readable and shortening line lengths.

Please drop the patch, if this not helping the code look better.

Thanks
Suniel

Re: [PATCH] staging: ccree: local variable "dev" not required

2017-10-05 Thread Suniel Mahesh 
On Thursday 05 October 2017 11:30 PM, Joe Perches wrote:
> On Thu, 2017-10-05 at 10:07 +0300, Gilad Ben-Yossef wrote:
>> On Wed, Oct 4, 2017 at 10:12 PM,   wrote:
>>> There is no need to create a local pointer variable "dev" and
>>> pass it various API's, instead use plat_dev which is enumerated
>>> by platform core on successful probe.
> []
>> I'm sorry but I don't understand what is the problem you are trying to solve 
>> or
>> what is the improvement suggested.

Hi Gilad and all,

Actually this patch is not a major improvement nor trying to solve some thing.

As struct device is a member of struct platform_device, I thought why can't we 
use
plat_dev (pointer to struct platform_device) through out the code. May be I was 
trying
to make code look compact, may be I am wrong.

>>
>> Why is having a local variable undesirable? I think having it makes
>> the code more readable, not less.
> 
> IMO: The local variable is _not_ undesirable.
>  It does make the code more readable and
>  shortens line lengths too.
> 

Yes, as you guys pointed out, we can use local variables. It is definitely 
making
the code more readable and shortening line lengths.

Please drop the patch, if this not helping the code look better.

Thanks
Suniel

Re: [PATCH v2] drivers: cpufreq: Fix sysfs duplicate filename creation for platform-device

2017-09-19 Thread Suniel Mahesh 
On Wednesday 20 September 2017 04:06 AM, Viresh Kumar wrote:
> On 19-09-17, 10:12, Dave Gerlach wrote:
>> Hi,
>> On 09/18/2017 02:18 PM, suni...@techveda.org wrote:
>>> From: Suniel Mahesh <suni...@techveda.org>
>>>
>>> ti-cpufreq and cpufreq-dt-platdev drivers are registering platform-device
>>> with same name "cpufreq-dt" using platform_device_register_*() routines.
>>> This is leading to build warnings appended below.
>>>
>>> Providing hardware information to OPP framework along with the platform-
>>> device creation should be done by ti-cpufreq driver before cpufreq-dt
>>> driver comes into place.
>>>
>>> This patch add's TI SoC am33xx (uses opp-v2 property) in the blacklist of
>>> devices in cpufreq-dt-platform driver to avoid creating platform-device
>>> twice and remove build warnings (suggested by Viresh Kumar).
>>
>> This looks good to me, but this also affects "ti,am43" and "ti,dra7" 
>> platforms,
>> care to add those to the blacklist as well with this patch? Thanks.

Thanks and will also add those platforms.
 
> 
> Also please rebase on top of the pm/linux-next branch as I have pushed
> a similar patch there. (Actually its applied to pm/bleeding-edge
> branch for now and may take a day to get to pm/linux-next. You can
> wait in that case.).
> 

Will check both pm/linux-next and pm/bleeding-edge branches, will wait 
in that case and then rebase. Thanks

Re: [PATCH v2] drivers: cpufreq: Fix sysfs duplicate filename creation for platform-device

2017-09-19 Thread Suniel Mahesh 
On Wednesday 20 September 2017 04:06 AM, Viresh Kumar wrote:
> On 19-09-17, 10:12, Dave Gerlach wrote:
>> Hi,
>> On 09/18/2017 02:18 PM, suni...@techveda.org wrote:
>>> From: Suniel Mahesh 
>>>
>>> ti-cpufreq and cpufreq-dt-platdev drivers are registering platform-device
>>> with same name "cpufreq-dt" using platform_device_register_*() routines.
>>> This is leading to build warnings appended below.
>>>
>>> Providing hardware information to OPP framework along with the platform-
>>> device creation should be done by ti-cpufreq driver before cpufreq-dt
>>> driver comes into place.
>>>
>>> This patch add's TI SoC am33xx (uses opp-v2 property) in the blacklist of
>>> devices in cpufreq-dt-platform driver to avoid creating platform-device
>>> twice and remove build warnings (suggested by Viresh Kumar).
>>
>> This looks good to me, but this also affects "ti,am43" and "ti,dra7" 
>> platforms,
>> care to add those to the blacklist as well with this patch? Thanks.

Thanks and will also add those platforms.
 
> 
> Also please rebase on top of the pm/linux-next branch as I have pushed
> a similar patch there. (Actually its applied to pm/bleeding-edge
> branch for now and may take a day to get to pm/linux-next. You can
> wait in that case.).
> 

Will check both pm/linux-next and pm/bleeding-edge branches, will wait 
in that case and then rebase. Thanks

Re: [PATCH] drivers: cpufreq: Fix sysfs duplicate filename creation for platform-device

2017-09-18 Thread Suniel Mahesh 
On Monday 18 September 2017 06:45 AM, Viresh Kumar wrote:
> On 17-09-17, 00:04, suni...@techveda.org wrote:
>> From: Suniel Mahesh <suni...@techveda.org>
>>
>> ti-cpufreq.c and cpufreq-dt-platdev.c are registering
>> platform device with same name "cpufreq-dt" using
>> platform_device_register_*() routines.
>> This is leading to build warnings appended below.
>>
>> This patch does the following:
>> (a) Remove platform-device registration routine from
>> ti-cpufreq.c, it appears unnecessary as per CPUfreq framework
>> kernel Documentation. The ti-cpufreq driver main task is to use
>> revision and an efuse value from the SoC and provide the OPP
>> framework with supported hardware information.
>> Platform-device creation is taken care by cpufreq-dt-platdev.c.
>> (b) In case if OPP-v2 is not supported just return without
>> registering the platorm-device.
>> (c) Rename the goto label
>>
>> [2.370167] [ cut here ]
>> [2.375087] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 
>> sysfs_warn_dup+0x58/0x78
>> [2.383112] sysfs: cannot create duplicate filename 
>> '/devices/platform/cpufreq-dt'
>> [2.391219] Modules linked in:
>> [2.394506] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 4.13.0-next-20170912 #1
>> [2.402006] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [2.408437] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>> [2.416568] [] (show_stack) from [] 
>> (dump_stack+0xac/0xe0)
>> [2.424165] [] (dump_stack) from [] 
>> (__warn+0xd8/0x104)
>> [2.431488] [] (__warn) from [] 
>> (warn_slowpath_fmt+0x34/0x44)
>> [2.439351] [] (warn_slowpath_fmt) from [] 
>> (sysfs_warn_dup+0x58/0x78)
>> [2.447938] [] (sysfs_warn_dup) from [] 
>> (sysfs_create_dir_ns+0x80/0x98)
>> [2.456719] [] (sysfs_create_dir_ns) from [] 
>> (kobject_add_internal+0x9c/0x2d4)
>> [2.466124] [] (kobject_add_internal) from [] 
>> (kobject_add+0x4c/0x9c)
>> [2.474712] [] (kobject_add) from [] 
>> (device_add+0xcc/0x57c)
>> [2.482489] [] (device_add) from [] 
>> (platform_device_add+0x100/0x220)
>> [2.491085] [] (platform_device_add) from [] 
>> (platform_device_register_full+0xf4/0x118)
>> [2.501305] [] (platform_device_register_full) from 
>> [] (ti_cpufreq_init+0x150/0x22c)
>> [2.511253] [] (ti_cpufreq_init) from [] 
>> (do_one_initcall+0x3c/0x170)
>> [2.519838] [] (do_one_initcall) from [] 
>> (kernel_init_freeable+0x1fc/0x2c4)
>> [2.528974] [] (kernel_init_freeable) from [] 
>> (kernel_init+0x8/0x110)
>> [2.537565] [] (kernel_init) from [] 
>> (ret_from_fork+0x14/0x3c)
>> [2.545981] ---[ end trace 2fc00e213c13ab20 ]---
>> [2.551051] [ cut here ]
>> [2.555931] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240 
>> kobject_add_internal+0x254/0x2d4
>> [2.564578] kobject_add_internal failed for cpufreq-dt with -EEXIST, 
>> don't try to register
>> things with the same name in the same directory.
>> [2.577977] Modules linked in:
>> [2.581261] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW   
>> 4.13.0-next-20170912 #1
>> [2.590013] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [2.596437] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>> [2.604573] [] (show_stack) from [] 
>> (dump_stack+0xac/0xe0)
>> [2.612172] [] (dump_stack) from [] 
>> (__warn+0xd8/0x104)
>> [2.619494] [] (__warn) from [] 
>> (warn_slowpath_fmt+0x34/0x44)
>> [2.627362] [] (warn_slowpath_fmt) from [] 
>> (kobject_add_internal+0x254/0x2d4)
>> [2.63] [] (kobject_add_internal) from [] 
>> (kobject_add+0x4c/0x9c)
>> [2.645255] [] (kobject_add) from [] 
>> (device_add+0xcc/0x57c)
>> [2.653027] [] (device_add) from [] 
>> (platform_device_add+0x100/0x220)
>> [2.661615] [] (platform_device_add) from [] 
>> (platform_device_register_full+0xf4/0x118)
>> [2.671833] [] (platform_device_register_full) from 
>> [] (ti_cpufreq_init+0x150/0x22c)
>> [2.681779] [] (ti_cpufreq_init) from [] 
>> (do_one_initcall+0x3c/0x170)
>> [2.690377] [] (do_one_initcall) from [] 
>> (kernel_init_freeable+0x1fc/0x2c4)
>> [2.699510] [] (kernel_init_freeable) from [] 
>> (kernel_init+0x8/0x110)
>> [2.708106] [] (kernel_init) from [] 
>> (ret_from_fork+0x14/0x3c)
>> [2.716217] ---[ end trace 2fc00e213c13ab21 ]---
>>
>> Sig

Re: [PATCH] drivers: cpufreq: Fix sysfs duplicate filename creation for platform-device

2017-09-18 Thread Suniel Mahesh 
On Monday 18 September 2017 06:45 AM, Viresh Kumar wrote:
> On 17-09-17, 00:04, suni...@techveda.org wrote:
>> From: Suniel Mahesh 
>>
>> ti-cpufreq.c and cpufreq-dt-platdev.c are registering
>> platform device with same name "cpufreq-dt" using
>> platform_device_register_*() routines.
>> This is leading to build warnings appended below.
>>
>> This patch does the following:
>> (a) Remove platform-device registration routine from
>> ti-cpufreq.c, it appears unnecessary as per CPUfreq framework
>> kernel Documentation. The ti-cpufreq driver main task is to use
>> revision and an efuse value from the SoC and provide the OPP
>> framework with supported hardware information.
>> Platform-device creation is taken care by cpufreq-dt-platdev.c.
>> (b) In case if OPP-v2 is not supported just return without
>> registering the platorm-device.
>> (c) Rename the goto label
>>
>> [2.370167] [ cut here ]
>> [2.375087] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 
>> sysfs_warn_dup+0x58/0x78
>> [2.383112] sysfs: cannot create duplicate filename 
>> '/devices/platform/cpufreq-dt'
>> [2.391219] Modules linked in:
>> [2.394506] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 4.13.0-next-20170912 #1
>> [2.402006] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [2.408437] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>> [2.416568] [] (show_stack) from [] 
>> (dump_stack+0xac/0xe0)
>> [2.424165] [] (dump_stack) from [] 
>> (__warn+0xd8/0x104)
>> [2.431488] [] (__warn) from [] 
>> (warn_slowpath_fmt+0x34/0x44)
>> [2.439351] [] (warn_slowpath_fmt) from [] 
>> (sysfs_warn_dup+0x58/0x78)
>> [2.447938] [] (sysfs_warn_dup) from [] 
>> (sysfs_create_dir_ns+0x80/0x98)
>> [2.456719] [] (sysfs_create_dir_ns) from [] 
>> (kobject_add_internal+0x9c/0x2d4)
>> [2.466124] [] (kobject_add_internal) from [] 
>> (kobject_add+0x4c/0x9c)
>> [2.474712] [] (kobject_add) from [] 
>> (device_add+0xcc/0x57c)
>> [2.482489] [] (device_add) from [] 
>> (platform_device_add+0x100/0x220)
>> [2.491085] [] (platform_device_add) from [] 
>> (platform_device_register_full+0xf4/0x118)
>> [2.501305] [] (platform_device_register_full) from 
>> [] (ti_cpufreq_init+0x150/0x22c)
>> [2.511253] [] (ti_cpufreq_init) from [] 
>> (do_one_initcall+0x3c/0x170)
>> [2.519838] [] (do_one_initcall) from [] 
>> (kernel_init_freeable+0x1fc/0x2c4)
>> [2.528974] [] (kernel_init_freeable) from [] 
>> (kernel_init+0x8/0x110)
>> [2.537565] [] (kernel_init) from [] 
>> (ret_from_fork+0x14/0x3c)
>> [2.545981] ---[ end trace 2fc00e213c13ab20 ]---
>> [2.551051] [ cut here ]
>> [2.555931] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240 
>> kobject_add_internal+0x254/0x2d4
>> [2.564578] kobject_add_internal failed for cpufreq-dt with -EEXIST, 
>> don't try to register
>> things with the same name in the same directory.
>> [2.577977] Modules linked in:
>> [2.581261] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW   
>> 4.13.0-next-20170912 #1
>> [2.590013] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [2.596437] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>> [2.604573] [] (show_stack) from [] 
>> (dump_stack+0xac/0xe0)
>> [2.612172] [] (dump_stack) from [] 
>> (__warn+0xd8/0x104)
>> [2.619494] [] (__warn) from [] 
>> (warn_slowpath_fmt+0x34/0x44)
>> [2.627362] [] (warn_slowpath_fmt) from [] 
>> (kobject_add_internal+0x254/0x2d4)
>> [2.63] [] (kobject_add_internal) from [] 
>> (kobject_add+0x4c/0x9c)
>> [2.645255] [] (kobject_add) from [] 
>> (device_add+0xcc/0x57c)
>> [2.653027] [] (device_add) from [] 
>> (platform_device_add+0x100/0x220)
>> [2.661615] [] (platform_device_add) from [] 
>> (platform_device_register_full+0xf4/0x118)
>> [2.671833] [] (platform_device_register_full) from 
>> [] (ti_cpufreq_init+0x150/0x22c)
>> [2.681779] [] (ti_cpufreq_init) from [] 
>> (do_one_initcall+0x3c/0x170)
>> [2.690377] [] (do_one_initcall) from [] 
>> (kernel_init_freeable+0x1fc/0x2c4)
>> [2.699510] [] (kernel_init_freeable) from [] 
>> (kernel_init+0x8/0x110)
>> [2.708106] [] (kernel_init) from [] 
>> (ret_from_fork+0x14/0x3c)
>> [2.716217] ---[ end trace 2fc00e213c13ab21 ]---
>>
>> Signed-off-by: Suniel Mahesh 
>>

Re: [PATCH 2/3] staging: ccree: Convert to devm_ioremap_resource for map, unmap

2017-07-27 Thread Suniel Mahesh 
On Friday 28 July 2017 01:18 AM, Dan Carpenter wrote:
> On Thu, Jul 27, 2017 at 05:27:33PM +0300, Gilad Ben-Yossef wrote:
>> +new_drvdata->cc_base = devm_ioremap_resource(_dev->dev,
>> + req_mem_cc_regs);
>> +if (IS_ERR(new_drvdata->cc_base)) {
>> +rc = PTR_ERR(new_drvdata->cc_base);
>>  goto init_cc_res_err;
> 
> (This code was in the original and not introduced by the patch.)

Hi Dan, the above lines of code were not in the original except 
"goto init_cc_res_err;" which was doing the error handling at different
places.

This patch has added those first three lines. I was constantly checking the 
latest
linux-next and staging-testing / next git trees. 
 
> 
> Ideally, the goto name should say what the goto does.  In this case it
> does everything.  Unfortunately trying to do everything is very
> complicated so obviously the error handling is going to be full of bugs.
> 
> The first thing the error handling does is:
> 
>   ssi_aead_free(new_drvdata);
> 
> But this function assumes that if new_drvdata->aead_handle is non-NULL
> then that means we have called:
> 
>   INIT_LIST_HEAD(_handle->aead_list);
> 
> That assumption is false if the aead_handle->sram_workspace_addr
> allocation fails.  It can't actually fail in the current code...  So
> that's good, I suppose.  Reviewing this code is really hard, because I
> have to jump back and forth through several functions in different
> files.
> 
> Moving on two the second error handling function:
> 
>   ssi_hash_free(new_drvdata);
> 
> This one has basically the same assumption that if ->hash_handle is
> allocated that means we called:
> 
>   INIT_LIST_HEAD(_handle->hash_list);
> 
> That assumption is not true if ssi_hash_init_sram_digest_consts(drvdata);
> fails.  That function can fail in real life.  Except the the error
> handling in ssi_hash_alloc() sets ->hash_handle to NULL.  So the bug is
> just a leak and not a crashing bug.
> 
> I've reviewed the first two lines of the error handling just to give a
> feel for how complicated "do everything" style error handling is to
> review.
> 
> The better way to do error handling is:
> 1) Only free things which have been allocated.
> 2) The unwind code should mirror the wind up code.
> 3) Every allocation function should have a free function.
> 4) Label names should tell you what the goto does.
> 5) Use direct returns and literals where possible.
> 6) Generally it's better to keep the error path and the success path
>separate.
> 7) Do error handling as opposed to success handling.
> 
>   one = alloc();
>   if (!one)
>   return -ENOMEM;
>   if (foo) {
>   two = alloc();
>   if (!two) {
>   ret = -ENOMEM;
>   goto free_one;
>   }
>   }
>   three = alloc();
>   if (!three) {
>   ret = -ENOMEM;
>   goto free_two;
>   }
>   ...
> 
>   return 0;
> 
> free_two:
>   if (foo)
>   free(two);
> free_one:
>   free(one);
> 
>   return ret;
> 
> This style of error handling is easier to review.  You only need to
> remember the most recent thing that you have allocated.  You can tell
> from the goto that it frees it so you don't have to scroll to the
> bottom of the function or jump to a different file.

I understand, its make sense, may be we should come up with something 
better and simpler.

Thanks
Suniel

> 
> regards,
> dan carpenter
> 
>

Re: [PATCH 2/3] staging: ccree: Convert to devm_ioremap_resource for map, unmap

2017-07-27 Thread Suniel Mahesh 
On Friday 28 July 2017 01:18 AM, Dan Carpenter wrote:
> On Thu, Jul 27, 2017 at 05:27:33PM +0300, Gilad Ben-Yossef wrote:
>> +new_drvdata->cc_base = devm_ioremap_resource(_dev->dev,
>> + req_mem_cc_regs);
>> +if (IS_ERR(new_drvdata->cc_base)) {
>> +rc = PTR_ERR(new_drvdata->cc_base);
>>  goto init_cc_res_err;
> 
> (This code was in the original and not introduced by the patch.)

Hi Dan, the above lines of code were not in the original except 
"goto init_cc_res_err;" which was doing the error handling at different
places.

This patch has added those first three lines. I was constantly checking the 
latest
linux-next and staging-testing / next git trees. 
 
> 
> Ideally, the goto name should say what the goto does.  In this case it
> does everything.  Unfortunately trying to do everything is very
> complicated so obviously the error handling is going to be full of bugs.
> 
> The first thing the error handling does is:
> 
>   ssi_aead_free(new_drvdata);
> 
> But this function assumes that if new_drvdata->aead_handle is non-NULL
> then that means we have called:
> 
>   INIT_LIST_HEAD(_handle->aead_list);
> 
> That assumption is false if the aead_handle->sram_workspace_addr
> allocation fails.  It can't actually fail in the current code...  So
> that's good, I suppose.  Reviewing this code is really hard, because I
> have to jump back and forth through several functions in different
> files.
> 
> Moving on two the second error handling function:
> 
>   ssi_hash_free(new_drvdata);
> 
> This one has basically the same assumption that if ->hash_handle is
> allocated that means we called:
> 
>   INIT_LIST_HEAD(_handle->hash_list);
> 
> That assumption is not true if ssi_hash_init_sram_digest_consts(drvdata);
> fails.  That function can fail in real life.  Except the the error
> handling in ssi_hash_alloc() sets ->hash_handle to NULL.  So the bug is
> just a leak and not a crashing bug.
> 
> I've reviewed the first two lines of the error handling just to give a
> feel for how complicated "do everything" style error handling is to
> review.
> 
> The better way to do error handling is:
> 1) Only free things which have been allocated.
> 2) The unwind code should mirror the wind up code.
> 3) Every allocation function should have a free function.
> 4) Label names should tell you what the goto does.
> 5) Use direct returns and literals where possible.
> 6) Generally it's better to keep the error path and the success path
>separate.
> 7) Do error handling as opposed to success handling.
> 
>   one = alloc();
>   if (!one)
>   return -ENOMEM;
>   if (foo) {
>   two = alloc();
>   if (!two) {
>   ret = -ENOMEM;
>   goto free_one;
>   }
>   }
>   three = alloc();
>   if (!three) {
>   ret = -ENOMEM;
>   goto free_two;
>   }
>   ...
> 
>   return 0;
> 
> free_two:
>   if (foo)
>   free(two);
> free_one:
>   free(one);
> 
>   return ret;
> 
> This style of error handling is easier to review.  You only need to
> remember the most recent thing that you have allocated.  You can tell
> from the goto that it frees it so you don't have to scroll to the
> bottom of the function or jump to a different file.

I understand, its make sense, may be we should come up with something 
better and simpler.

Thanks
Suniel

> 
> regards,
> dan carpenter
> 
>

Re: [PATCH 1/3] staging: ccree: Replace kzalloc with devm_kzalloc

2017-07-17 Thread Suniel Mahesh 
On Monday 17 July 2017 06:03 PM, Greg KH wrote:
> On Sat, Jul 15, 2017 at 01:21:54PM +0530, suni...@techveda.org wrote:
>> From: Suniel Mahesh <suni...@techveda.org>
>>
>> It is recommended to use managed function devm_kzalloc, which
>> simplifies driver cleanup paths and driver code.
>> This patch does the following:
>> (a) replace kzalloc with devm_kzalloc.
>> (b) drop kfree(), because memory allocated with devm_kzalloc() is
>> automatically freed on driver detach, otherwise it leads to a double
>> free.
>> (c) remove unnecessary blank lines.
>>
>> Signed-off-by: Suniel Mahesh <suni...@techveda.org>
>> ---
>> Note:
>> - Patch was tested and built(ARCH=arm) on next-20170714.
>>   No build issues reported, however it was not tested on
>>   real hardware.
>> ---
>>  drivers/staging/ccree/ssi_driver.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_driver.c 
>> b/drivers/staging/ccree/ssi_driver.c
>> index 78709b92..f231ecf 100644
>> --- a/drivers/staging/ccree/ssi_driver.c
>> +++ b/drivers/staging/ccree/ssi_driver.c
>> @@ -224,13 +224,15 @@ static int init_cc_resources(struct platform_device 
>> *plat_dev)
>>  struct resource *req_mem_cc_regs = NULL;
>>  void __iomem *cc_base = NULL;
>>  bool irq_registered = false;
>> -struct ssi_drvdata *new_drvdata = kzalloc(sizeof(struct ssi_drvdata), 
>> GFP_KERNEL);
>> +struct ssi_drvdata *new_drvdata;
>>  struct device *dev = _dev->dev;
>>  struct device_node *np = dev->of_node;
>>  u32 signature_val;
>>  int rc = 0;
>>  
>> -if (unlikely(!new_drvdata)) {
>> +new_drvdata = devm_kzalloc(_dev->dev, sizeof(struct ssi_drvdata),
> 
> sizeof(*new_drvdata), right?
> 
Yes, sending v2 in a while. Thanks for the review
suniel
> thanks,
> 
> greg k-h
>

Re: [PATCH 1/3] staging: ccree: Replace kzalloc with devm_kzalloc

2017-07-17 Thread Suniel Mahesh 
On Monday 17 July 2017 06:03 PM, Greg KH wrote:
> On Sat, Jul 15, 2017 at 01:21:54PM +0530, suni...@techveda.org wrote:
>> From: Suniel Mahesh 
>>
>> It is recommended to use managed function devm_kzalloc, which
>> simplifies driver cleanup paths and driver code.
>> This patch does the following:
>> (a) replace kzalloc with devm_kzalloc.
>> (b) drop kfree(), because memory allocated with devm_kzalloc() is
>> automatically freed on driver detach, otherwise it leads to a double
>> free.
>> (c) remove unnecessary blank lines.
>>
>> Signed-off-by: Suniel Mahesh 
>> ---
>> Note:
>> - Patch was tested and built(ARCH=arm) on next-20170714.
>>   No build issues reported, however it was not tested on
>>   real hardware.
>> ---
>>  drivers/staging/ccree/ssi_driver.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_driver.c 
>> b/drivers/staging/ccree/ssi_driver.c
>> index 78709b92..f231ecf 100644
>> --- a/drivers/staging/ccree/ssi_driver.c
>> +++ b/drivers/staging/ccree/ssi_driver.c
>> @@ -224,13 +224,15 @@ static int init_cc_resources(struct platform_device 
>> *plat_dev)
>>  struct resource *req_mem_cc_regs = NULL;
>>  void __iomem *cc_base = NULL;
>>  bool irq_registered = false;
>> -struct ssi_drvdata *new_drvdata = kzalloc(sizeof(struct ssi_drvdata), 
>> GFP_KERNEL);
>> +struct ssi_drvdata *new_drvdata;
>>  struct device *dev = _dev->dev;
>>  struct device_node *np = dev->of_node;
>>  u32 signature_val;
>>  int rc = 0;
>>  
>> -if (unlikely(!new_drvdata)) {
>> +new_drvdata = devm_kzalloc(_dev->dev, sizeof(struct ssi_drvdata),
> 
> sizeof(*new_drvdata), right?
> 
Yes, sending v2 in a while. Thanks for the review
suniel
> thanks,
> 
> greg k-h
>

Re: [PATCH] arm: dts: omap3: Remove interrupt-parent property

2017-06-26 Thread Suniel Mahesh 
On Tuesday 27 June 2017 11:03 AM, kart...@techveda.org wrote:
> From: Karthik Tummala 
> 
Please change the subject of the patch to something which is more meaningful 
and 
which reflect the changes done to the code base.
> All nodes inhert "interrupt-parent" property from root
> node. So removed that property from usbhsohci, usbhsehci,
> ssi_port1, ssi_port2 nodes.
Adjust description accordingly.
> 
> Signed-off-by: Karthik Tummala 
> ---
>  arch/arm/boot/dts/omap3.dtsi | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index a3ff493..bdaf30c 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -713,14 +713,12 @@
>   usbhsohci: ohci@48064400 {
>   compatible = "ti,ohci-omap3";
>   reg = <0x48064400 0x400>;
> - interrupt-parent = <>;
>   interrupts = <76>;
>   };
>  
>   usbhsehci: ehci@48064800 {
>   compatible = "ti,ehci-omap";
>   reg = <0x48064800 0x400>;
> - interrupt-parent = <>;
>   interrupts = <77>;
>   };
>   };
> @@ -831,7 +829,6 @@
>   reg-names = "tx",
>   "rx";
>  
> - interrupt-parent = <>;
>   interrupts = <67>,
><68>;
>   };
> @@ -844,7 +841,6 @@
>   reg-names = "tx",
>   "rx";
>  
> - interrupt-parent = <>;
>   interrupts = <69>,
><70>;
>   };
> 

Thanks
Suniel

Re: [PATCH] arm: dts: omap3: Remove interrupt-parent property

2017-06-26 Thread Suniel Mahesh 
On Tuesday 27 June 2017 11:03 AM, kart...@techveda.org wrote:
> From: Karthik Tummala 
> 
Please change the subject of the patch to something which is more meaningful 
and 
which reflect the changes done to the code base.
> All nodes inhert "interrupt-parent" property from root
> node. So removed that property from usbhsohci, usbhsehci,
> ssi_port1, ssi_port2 nodes.
Adjust description accordingly.
> 
> Signed-off-by: Karthik Tummala 
> ---
>  arch/arm/boot/dts/omap3.dtsi | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index a3ff493..bdaf30c 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -713,14 +713,12 @@
>   usbhsohci: ohci@48064400 {
>   compatible = "ti,ohci-omap3";
>   reg = <0x48064400 0x400>;
> - interrupt-parent = <>;
>   interrupts = <76>;
>   };
>  
>   usbhsehci: ehci@48064800 {
>   compatible = "ti,ehci-omap";
>   reg = <0x48064800 0x400>;
> - interrupt-parent = <>;
>   interrupts = <77>;
>   };
>   };
> @@ -831,7 +829,6 @@
>   reg-names = "tx",
>   "rx";
>  
> - interrupt-parent = <>;
>   interrupts = <67>,
><68>;
>   };
> @@ -844,7 +841,6 @@
>   reg-names = "tx",
>   "rx";
>  
> - interrupt-parent = <>;
>   interrupts = <69>,
><70>;
>   };
> 

Thanks
Suniel

Re: [PATCH v2 5/5] staging: rtl8192e: Pass a pointer as an argument to sizeof() instead of struct

2017-03-17 Thread Suniel Mahesh 

On Friday 10 March 2017 12:20 AM, suni...@techveda.org wrote:

From: Suniel Mahesh <suni...@techveda.org>

Prefer vzalloc(sizeof(*priv->pFirmware)...) over
vzalloc(sizeof(struct rt_firmware)...) as reported by checkpatch.pl

Signed-off-by: Suniel Mahesh <suni...@techveda.org>
---
Changes for v2:

- new patch addition to the series
- Rebased on top of next-20170306
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index fb711d2..a099bce 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -965,7 +965,7 @@ static void _rtl92e_init_priv_variable(struct net_device 
*dev)

priv->card_type = PCI;

-   priv->pFirmware = vzalloc(sizeof(struct rt_firmware));
+   priv->pFirmware = vzalloc(sizeof(*priv->pFirmware));
if (!priv->pFirmware)
return -ENOMEM;
skb_queue_head_init(>skb_queue);


ok

Re: [PATCH v2 5/5] staging: rtl8192e: Pass a pointer as an argument to sizeof() instead of struct

2017-03-17 Thread Suniel Mahesh 

On Friday 10 March 2017 12:20 AM, suni...@techveda.org wrote:

From: Suniel Mahesh 

Prefer vzalloc(sizeof(*priv->pFirmware)...) over
vzalloc(sizeof(struct rt_firmware)...) as reported by checkpatch.pl

Signed-off-by: Suniel Mahesh 
---
Changes for v2:

- new patch addition to the series
- Rebased on top of next-20170306
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index fb711d2..a099bce 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -965,7 +965,7 @@ static void _rtl92e_init_priv_variable(struct net_device 
*dev)

priv->card_type = PCI;

-   priv->pFirmware = vzalloc(sizeof(struct rt_firmware));
+   priv->pFirmware = vzalloc(sizeof(*priv->pFirmware));
if (!priv->pFirmware)
return -ENOMEM;
skb_queue_head_init(>skb_queue);


ok

Re: [PATCH v3 2/8] staging: rtl8192e: Fix coding style

2017-03-15 Thread Suniel Mahesh 

On Wednesday 15 March 2017 03:44 PM, Dan Carpenter wrote:

On Wed, Mar 15, 2017 at 03:21:51PM +0530, suni...@techveda.org wrote:

@@ -1796,7 +1796,7 @@ static short _rtl92e_alloc_rx_ring(struct net_device *dev)

for (rx_queue_idx = 0; rx_queue_idx < MAX_RX_QUEUE; rx_queue_idx++) {
priv->rx_ring[rx_queue_idx] = pci_zalloc_consistent(priv->pdev,
- sizeof(*priv->rx_ring[rx_queue_idx]) 
* priv->rxringcount,
+   sizeof(*priv->rx_ring[rx_queue_idx]) * priv->rxringcount,
  >rx_ring_dma[rx_queue_idx]);


No, don't do that.  The original was easier to read.  Ignore
checkpatch.pl if it gives you bad advice.


if (!priv->rx_ring[rx_queue_idx] ||
(unsigned long)priv->rx_ring[rx_queue_idx] & 0xFF) {
@@ -2272,7 +2272,8 @@ static int _rtl92e_ioctl(struct net_device *dev, struct 
ifreq *rq, int cmd)
int ret = -1;
struct rtllib_device *ieee = priv->rtllib;
u32 key[4];
-   const u8 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 
0xff};
+   const u8 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff,
+   0xff};


Just drop this patch...  The original is better.

regards,
dan carpenter

hi, when you say drop this patch, should I send the entire patch set as 
PATCH v4 with this particular patch dropped ?


regards
suniel

Re: [PATCH v3 2/8] staging: rtl8192e: Fix coding style

2017-03-15 Thread Suniel Mahesh 

On Wednesday 15 March 2017 03:44 PM, Dan Carpenter wrote:

On Wed, Mar 15, 2017 at 03:21:51PM +0530, suni...@techveda.org wrote:

@@ -1796,7 +1796,7 @@ static short _rtl92e_alloc_rx_ring(struct net_device *dev)

for (rx_queue_idx = 0; rx_queue_idx < MAX_RX_QUEUE; rx_queue_idx++) {
priv->rx_ring[rx_queue_idx] = pci_zalloc_consistent(priv->pdev,
- sizeof(*priv->rx_ring[rx_queue_idx]) 
* priv->rxringcount,
+   sizeof(*priv->rx_ring[rx_queue_idx]) * priv->rxringcount,
  >rx_ring_dma[rx_queue_idx]);


No, don't do that.  The original was easier to read.  Ignore
checkpatch.pl if it gives you bad advice.


if (!priv->rx_ring[rx_queue_idx] ||
(unsigned long)priv->rx_ring[rx_queue_idx] & 0xFF) {
@@ -2272,7 +2272,8 @@ static int _rtl92e_ioctl(struct net_device *dev, struct 
ifreq *rq, int cmd)
int ret = -1;
struct rtllib_device *ieee = priv->rtllib;
u32 key[4];
-   const u8 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 
0xff};
+   const u8 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff,
+   0xff};


Just drop this patch...  The original is better.

regards,
dan carpenter

hi, when you say drop this patch, should I send the entire patch set as 
PATCH v4 with this particular patch dropped ?


regards
suniel

[PATCH] staging: rtl8192e: fix coding style issue, improve error handling

2017-03-06 Thread Suniel Mahesh 
Fix coding style issue and comments in rtl_core.c
Return -ENOMEM, if it is out of memory
Pointer comparison with NULL replaced by logical NOT

Signed-off-by: Suniel Mahesh <suni...@techveda.org>
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 253 +++
 1 file changed, 100 insertions(+), 153 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 4c0caa6..1099c94 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -1,4 +1,4 @@
-/**
+/
  * Copyright(c) 2008 - 2010 Realtek Corporation. All rights reserved.
  *
  * Based on the r8180 driver, which is:
@@ -17,7 +17,7 @@
  *
  * Contact Information:
  * wlanfae <wlan...@realtek.com>
-**/
+ */
 #include 
 #include 
 #include 
@@ -37,7 +37,6 @@
 static int channels = 0x3fff;
 static char *ifname = "wlan%d";
 
-
 static const struct rtl819x_ops rtl819xp_ops = {
.nic_type   = NIC_8192E,
.get_eeprom_size= rtl92e_get_eeprom_size,
@@ -100,9 +99,7 @@ static void _rtl92e_hard_data_xmit(struct sk_buff *skb, 
struct net_device *dev,
 static int _rtl92e_down(struct net_device *dev, bool shutdownrf);
 static void _rtl92e_restart(void *data);
 
-/
-   -IO STUFF-
-*/
+/* IO STUFF */
 
 u8 rtl92e_readb(struct net_device *dev, int x)
 {
@@ -140,9 +137,7 @@ void rtl92e_writew(struct net_device *dev, int x, u16 y)
udelay(20);
 }
 
-/
-   -GENERAL FUNCTION-
-*/
+/* GENERAL FUNCTION */
 bool rtl92e_set_rf_state(struct net_device *dev,
 enum rt_rf_power_state StateToSet,
 RT_RF_CHANGE_SOURCE ChangeSource)
@@ -200,7 +195,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
priv->rtllib->RfOffReason = 0;
bActionAllowed = true;
 
-
if (rtState == eRfOff &&
ChangeSource >= RF_CHANGE_BY_HW)
bConnectBySSID = true;
@@ -223,7 +217,8 @@ bool rtl92e_set_rf_state(struct net_device *dev,
else
priv->blinked_ingpio = false;
rtllib_MgntDisconnect(priv->rtllib,
- 
WLAN_REASON_DISASSOC_STA_HAS_LEFT);
+ WLAN_REASON_DISASSOC_STA_
+   HAS_LEFT);
}
}
if ((ChangeSource == RF_CHANGE_BY_HW) && !priv->bHwRadioOff)
@@ -247,7 +242,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
 StateToSet, priv->rtllib->RfOffReason);
PHY_SetRFPowerState(dev, StateToSet);
if (StateToSet == eRfOn) {
-
if (bConnectBySSID && priv->blinked_ingpio) {
schedule_delayed_work(
 >associate_procedure_wq, 0);
@@ -346,16 +340,16 @@ static void _rtl92e_update_cap(struct net_device *dev, 
u16 cap)
}
}
 
-   if (net->mode & (IEEE_G|IEEE_N_24G)) {
+   if (net->mode & (IEEE_G | IEEE_N_24G)) {
u8  slot_time_val;
u8  CurSlotTime = priv->slot_time;
 
if ((cap & WLAN_CAPABILITY_SHORT_SLOT_TIME) &&
-  (!priv->rtllib->pHTInfo->bCurrentRT2RTLongSlotTime)) {
+   (!priv->rtllib->pHTInfo->bCurrentRT2RTLongSlotTime)) {
if (CurSlotTime != SHORT_SLOT_TIME) {
slot_time_val = SHORT_SLOT_TIME;
priv->rtllib->SetHwRegHandler(dev,
-HW_VAR_SLOT_TIME, _time_val);
+   HW_VAR_SLOT_TIME, _time_val);
}
} else {
if (CurSlotTime != NON_SHORT_SLOT_TIME) {
@@ -407,7 +401,6 @@ static void _rtl92e_qos_activate(void *data)
for (i = 0; i <  QOS_QUEU

[PATCH] staging: rtl8192e: fix coding style issue, improve error handling

2017-03-06 Thread Suniel Mahesh 
Fix coding style issue and comments in rtl_core.c
Return -ENOMEM, if it is out of memory
Pointer comparison with NULL replaced by logical NOT

Signed-off-by: Suniel Mahesh 
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 253 +++
 1 file changed, 100 insertions(+), 153 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 4c0caa6..1099c94 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -1,4 +1,4 @@
-/**
+/
  * Copyright(c) 2008 - 2010 Realtek Corporation. All rights reserved.
  *
  * Based on the r8180 driver, which is:
@@ -17,7 +17,7 @@
  *
  * Contact Information:
  * wlanfae 
-**/
+ */
 #include 
 #include 
 #include 
@@ -37,7 +37,6 @@
 static int channels = 0x3fff;
 static char *ifname = "wlan%d";
 
-
 static const struct rtl819x_ops rtl819xp_ops = {
.nic_type   = NIC_8192E,
.get_eeprom_size= rtl92e_get_eeprom_size,
@@ -100,9 +99,7 @@ static void _rtl92e_hard_data_xmit(struct sk_buff *skb, 
struct net_device *dev,
 static int _rtl92e_down(struct net_device *dev, bool shutdownrf);
 static void _rtl92e_restart(void *data);
 
-/
-   -IO STUFF-
-*/
+/* IO STUFF */
 
 u8 rtl92e_readb(struct net_device *dev, int x)
 {
@@ -140,9 +137,7 @@ void rtl92e_writew(struct net_device *dev, int x, u16 y)
udelay(20);
 }
 
-/
-   -GENERAL FUNCTION-
-*/
+/* GENERAL FUNCTION */
 bool rtl92e_set_rf_state(struct net_device *dev,
 enum rt_rf_power_state StateToSet,
 RT_RF_CHANGE_SOURCE ChangeSource)
@@ -200,7 +195,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
priv->rtllib->RfOffReason = 0;
bActionAllowed = true;
 
-
if (rtState == eRfOff &&
ChangeSource >= RF_CHANGE_BY_HW)
bConnectBySSID = true;
@@ -223,7 +217,8 @@ bool rtl92e_set_rf_state(struct net_device *dev,
else
priv->blinked_ingpio = false;
rtllib_MgntDisconnect(priv->rtllib,
- 
WLAN_REASON_DISASSOC_STA_HAS_LEFT);
+ WLAN_REASON_DISASSOC_STA_
+   HAS_LEFT);
}
}
if ((ChangeSource == RF_CHANGE_BY_HW) && !priv->bHwRadioOff)
@@ -247,7 +242,6 @@ bool rtl92e_set_rf_state(struct net_device *dev,
 StateToSet, priv->rtllib->RfOffReason);
PHY_SetRFPowerState(dev, StateToSet);
if (StateToSet == eRfOn) {
-
if (bConnectBySSID && priv->blinked_ingpio) {
schedule_delayed_work(
 >associate_procedure_wq, 0);
@@ -346,16 +340,16 @@ static void _rtl92e_update_cap(struct net_device *dev, 
u16 cap)
}
}
 
-   if (net->mode & (IEEE_G|IEEE_N_24G)) {
+   if (net->mode & (IEEE_G | IEEE_N_24G)) {
u8  slot_time_val;
u8  CurSlotTime = priv->slot_time;
 
if ((cap & WLAN_CAPABILITY_SHORT_SLOT_TIME) &&
-  (!priv->rtllib->pHTInfo->bCurrentRT2RTLongSlotTime)) {
+   (!priv->rtllib->pHTInfo->bCurrentRT2RTLongSlotTime)) {
if (CurSlotTime != SHORT_SLOT_TIME) {
slot_time_val = SHORT_SLOT_TIME;
priv->rtllib->SetHwRegHandler(dev,
-HW_VAR_SLOT_TIME, _time_val);
+   HW_VAR_SLOT_TIME, _time_val);
}
} else {
if (CurSlotTime != NON_SHORT_SLOT_TIME) {
@@ -407,7 +401,6 @@ static void _rtl92e_qos_activate(void *data)
for (i = 0; i <  QOS_QUEUE_NUM; i++)
priv->rtllib->Se