[PATCH] bindings: net: stmmac: correct names of AXI properties

2019-01-09 Thread Jongsung Kim
Signed-off-by: Jongsung Kim 
---
 Documentation/devicetree/bindings/net/stmmac.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index cb694062afff..988e56781fbd 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -68,11 +68,11 @@ Optional properties:
- snps,xit_frm: unlock on WoL
- snps,wr_osr_lmt: max write outstanding req. limit
- snps,rd_osr_lmt: max read outstanding req. limit
-   - snps,kbbe: do not cross 1KiB boundary.
+   - snps,axi_kbbe: do not cross 1KiB boundary.
- snps,blen: this is a vector of supported burst length.
-   - snps,fb: fixed-burst
-   - snps,mb: mixed-burst
-   - snps,rb: rebuild INCRx Burst
+   - snps,axi_fb: fixed-burst
+   - snps,axi_mb: mixed-burst
+   - snps,axi_rb: rebuild INCRx Burst
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
 - Multiple RX Queues parameters: below the list of all the parameters to
 configure the multiple RX queues:
-- 
2.20.1



Re: [PATCH v3] watchdog: sp805: add restart handler

2018-05-08 Thread Jongsung Kim
On 05/04/2018 10:11 PM, Guenter Roeck wrote:
> On 05/03/2018 11:05 PM, Jongsung Kim wrote:
>> Add restart handler for SP805 watchdog so that the driver can be
>> used to reboot the system.
>>
>> Signed-off-by: Jongsung Kim <neidhard@lge.com>
>> Cc: Guenter Roeck <li...@roeck-us.net>
>
> Reviewed-by: Guenter Roeck <li...@roeck-us.net>
>
>> ---
>
> For future patches: change log goes here, please.
>
> Thanks,
> Guenter

Thank you for your review and kind comments.
JS

>
>>   drivers/watchdog/sp805_wdt.c | 14 ++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 03805bc..9849db0 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
>> *wdd)
>>   return div_u64(load, rate);
>>   }
>>   +static int
>> +wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
>> +{
>> +    struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +
>> +    writel_relaxed(0, wdt->base + WDTCONTROL);
>> +    writel_relaxed(0, wdt->base + WDTLOAD);
>> +    writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
>> +
>> +    return 0;
>> +}
>> +
>>   static int wdt_config(struct watchdog_device *wdd, bool ping)
>>   {
>>   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> @@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
>>   .ping    = wdt_ping,
>>   .set_timeout    = wdt_setload,
>>   .get_timeleft    = wdt_timeleft,
>> +    .restart    = wdt_restart,
>>   };
>>     static int
>> @@ -230,6 +243,7 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
>> amba_id *id)
>>   spin_lock_init(>lock);
>>   watchdog_set_nowayout(>wdd, nowayout);
>>   watchdog_set_drvdata(>wdd, wdt);
>> +    watchdog_set_restart_priority(>wdd, 128);
>>   wdt_setload(>wdd, DEFAULT_TIMEOUT);
>>     ret = watchdog_register_device(>wdd);
>>
>
>



Re: [PATCH v3] watchdog: sp805: add restart handler

2018-05-08 Thread Jongsung Kim
On 05/04/2018 10:11 PM, Guenter Roeck wrote:
> On 05/03/2018 11:05 PM, Jongsung Kim wrote:
>> Add restart handler for SP805 watchdog so that the driver can be
>> used to reboot the system.
>>
>> Signed-off-by: Jongsung Kim 
>> Cc: Guenter Roeck 
>
> Reviewed-by: Guenter Roeck 
>
>> ---
>
> For future patches: change log goes here, please.
>
> Thanks,
> Guenter

Thank you for your review and kind comments.
JS

>
>>   drivers/watchdog/sp805_wdt.c | 14 ++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 03805bc..9849db0 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
>> *wdd)
>>   return div_u64(load, rate);
>>   }
>>   +static int
>> +wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
>> +{
>> +    struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +
>> +    writel_relaxed(0, wdt->base + WDTCONTROL);
>> +    writel_relaxed(0, wdt->base + WDTLOAD);
>> +    writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
>> +
>> +    return 0;
>> +}
>> +
>>   static int wdt_config(struct watchdog_device *wdd, bool ping)
>>   {
>>   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> @@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
>>   .ping    = wdt_ping,
>>   .set_timeout    = wdt_setload,
>>   .get_timeleft    = wdt_timeleft,
>> +    .restart    = wdt_restart,
>>   };
>>     static int
>> @@ -230,6 +243,7 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
>> amba_id *id)
>>   spin_lock_init(>lock);
>>   watchdog_set_nowayout(>wdd, nowayout);
>>   watchdog_set_drvdata(>wdd, wdt);
>> +    watchdog_set_restart_priority(>wdd, 128);
>>   wdt_setload(>wdd, DEFAULT_TIMEOUT);
>>     ret = watchdog_register_device(>wdd);
>>
>
>



[PATCH v3] watchdog: sp805: add restart handler

2018-05-04 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Cc: Guenter Roeck <li...@roeck-us.net>
---
 drivers/watchdog/sp805_wdt.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..9849db0 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
*wdd)
return div_u64(load, rate);
 }
 
+static int
+wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int wdt_config(struct watchdog_device *wdd, bool ping)
 {
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
@@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
.ping   = wdt_ping,
.set_timeout= wdt_setload,
.get_timeleft   = wdt_timeleft,
+   .restart= wdt_restart,
 };
 
 static int
@@ -230,6 +243,7 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
amba_id *id)
spin_lock_init(>lock);
watchdog_set_nowayout(>wdd, nowayout);
watchdog_set_drvdata(>wdd, wdt);
+   watchdog_set_restart_priority(>wdd, 128);
wdt_setload(>wdd, DEFAULT_TIMEOUT);
 
ret = watchdog_register_device(>wdd);
-- 
2.7.4



[PATCH v3] watchdog: sp805: add restart handler

2018-05-04 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim 
Cc: Guenter Roeck 
---
 drivers/watchdog/sp805_wdt.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..9849db0 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
*wdd)
return div_u64(load, rate);
 }
 
+static int
+wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int wdt_config(struct watchdog_device *wdd, bool ping)
 {
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
@@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
.ping   = wdt_ping,
.set_timeout= wdt_setload,
.get_timeleft   = wdt_timeleft,
+   .restart= wdt_restart,
 };
 
 static int
@@ -230,6 +243,7 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
amba_id *id)
spin_lock_init(>lock);
watchdog_set_nowayout(>wdd, nowayout);
watchdog_set_drvdata(>wdd, wdt);
+   watchdog_set_restart_priority(>wdd, 128);
wdt_setload(>wdd, DEFAULT_TIMEOUT);
 
ret = watchdog_register_device(>wdd);
-- 
2.7.4



[PATCH v2] watchdog: sp805: add restart handler

2018-05-02 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Cc: Guenter Roeck <li...@roeck-us.net>
---
 drivers/watchdog/sp805_wdt.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..969369e 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
*wdd)
return div_u64(load, rate);
 }
 
+static int
+wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int wdt_config(struct watchdog_device *wdd, bool ping)
 {
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
@@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
.ping   = wdt_ping,
.set_timeout= wdt_setload,
.get_timeleft   = wdt_timeleft,
+   .restart= wdt_restart,
 };
 
 static int
-- 
2.7.4



[PATCH v2] watchdog: sp805: add restart handler

2018-05-02 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim 
Cc: Guenter Roeck 
---
 drivers/watchdog/sp805_wdt.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..969369e 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -121,6 +121,18 @@ static unsigned int wdt_timeleft(struct watchdog_device 
*wdd)
return div_u64(load, rate);
 }
 
+static int
+wdt_restart(struct watchdog_device *wdd, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int wdt_config(struct watchdog_device *wdd, bool ping)
 {
struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
@@ -197,6 +209,7 @@ static const struct watchdog_ops wdt_ops = {
.ping   = wdt_ping,
.set_timeout= wdt_setload,
.get_timeleft   = wdt_timeleft,
+   .restart= wdt_restart,
 };
 
 static int
-- 
2.7.4



Re: [PATCH] watchdog: sp805: add restart handler

2018-05-02 Thread Jongsung Kim
On 04/30/2018 08:18 PM, Guenter Roeck wrote:
> On 04/29/2018 11:44 PM, Jongsung Kim wrote:
>> Add restart handler for SP805 watchdog so that the driver can be
>> used to reboot the system.
>>
>> Signed-off-by: Jongsung Kim <neidhard@lge.com>
>> ---
>>   drivers/watchdog/sp805_wdt.c | 19 +++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 01d816251302..01f7b6c29f92 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -23,6 +23,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -67,6 +68,7 @@ struct sp805_wdt {
>>   struct clk    *clk;
>>   struct amba_device    *adev;
>>   unsigned int    load_val;
>> +    struct notifier_block    restart;
>>   };
>>     static bool nowayout = WATCHDOG_NOWAYOUT;
>> @@ -200,6 +202,18 @@ static const struct watchdog_ops wdt_ops = {
>>   .get_timeleft    = wdt_timeleft,
>>   };
>>   +static int
>> +wdt_restart(struct notifier_block *this, unsigned long mode, void *cmd)
>> +{
>> +    struct sp805_wdt *wdt = container_of(this, struct sp805_wdt, restart);
>> +
>> +    writel_relaxed(0, wdt->base + WDTCONTROL);
>> +    writel_relaxed(0, wdt->base + WDTLOAD);
>> +    writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
>> +
>> +    return 0;
>> +}
>> +
>>   static int
>>   sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>>   {
>> @@ -241,6 +255,10 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
>> amba_id *id)
>>   }
>>   amba_set_drvdata(adev, wdt);
>>   +    wdt->restart.notifier_call = wdt_restart;
>> +    wdt->restart.priority = 128;
>> +    register_restart_handler(>restart);
>> +
>
> Why not use the watchdog core ?

Thank you for pointing this. I didn't noticed core changes about restart 
function because I'm still using old v4.4.xx..

>
> Guenter
>
>>   dev_info(>dev, "registration successful\n");
>>   return 0;
>>   @@ -253,6 +271,7 @@ static int sp805_wdt_remove(struct amba_device *adev)
>>   {
>>   struct sp805_wdt *wdt = amba_get_drvdata(adev);
>>   +    unregister_restart_handler(>restart);
>>   watchdog_unregister_device(>wdd);
>>   watchdog_set_drvdata(>wdd, NULL);
>>  
>
>



Re: [PATCH] watchdog: sp805: add restart handler

2018-05-02 Thread Jongsung Kim
On 04/30/2018 08:18 PM, Guenter Roeck wrote:
> On 04/29/2018 11:44 PM, Jongsung Kim wrote:
>> Add restart handler for SP805 watchdog so that the driver can be
>> used to reboot the system.
>>
>> Signed-off-by: Jongsung Kim 
>> ---
>>   drivers/watchdog/sp805_wdt.c | 19 +++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 01d816251302..01f7b6c29f92 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -23,6 +23,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -67,6 +68,7 @@ struct sp805_wdt {
>>   struct clk    *clk;
>>   struct amba_device    *adev;
>>   unsigned int    load_val;
>> +    struct notifier_block    restart;
>>   };
>>     static bool nowayout = WATCHDOG_NOWAYOUT;
>> @@ -200,6 +202,18 @@ static const struct watchdog_ops wdt_ops = {
>>   .get_timeleft    = wdt_timeleft,
>>   };
>>   +static int
>> +wdt_restart(struct notifier_block *this, unsigned long mode, void *cmd)
>> +{
>> +    struct sp805_wdt *wdt = container_of(this, struct sp805_wdt, restart);
>> +
>> +    writel_relaxed(0, wdt->base + WDTCONTROL);
>> +    writel_relaxed(0, wdt->base + WDTLOAD);
>> +    writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
>> +
>> +    return 0;
>> +}
>> +
>>   static int
>>   sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>>   {
>> @@ -241,6 +255,10 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
>> amba_id *id)
>>   }
>>   amba_set_drvdata(adev, wdt);
>>   +    wdt->restart.notifier_call = wdt_restart;
>> +    wdt->restart.priority = 128;
>> +    register_restart_handler(>restart);
>> +
>
> Why not use the watchdog core ?

Thank you for pointing this. I didn't noticed core changes about restart 
function because I'm still using old v4.4.xx..

>
> Guenter
>
>>   dev_info(>dev, "registration successful\n");
>>   return 0;
>>   @@ -253,6 +271,7 @@ static int sp805_wdt_remove(struct amba_device *adev)
>>   {
>>   struct sp805_wdt *wdt = amba_get_drvdata(adev);
>>   +    unregister_restart_handler(>restart);
>>   watchdog_unregister_device(>wdd);
>>   watchdog_set_drvdata(>wdd, NULL);
>>  
>
>



[PATCH] watchdog: sp805: add restart handler

2018-04-30 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim <neidhard@lge.com>
---
 drivers/watchdog/sp805_wdt.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 01d816251302..01f7b6c29f92 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,6 +68,7 @@ struct sp805_wdt {
struct clk  *clk;
struct amba_device  *adev;
unsigned intload_val;
+   struct notifier_block   restart;
 };
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -200,6 +202,18 @@ static const struct watchdog_ops wdt_ops = {
.get_timeleft   = wdt_timeleft,
 };
 
+static int
+wdt_restart(struct notifier_block *this, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = container_of(this, struct sp805_wdt, restart);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int
 sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 {
@@ -241,6 +255,10 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
amba_id *id)
}
amba_set_drvdata(adev, wdt);
 
+   wdt->restart.notifier_call = wdt_restart;
+   wdt->restart.priority = 128;
+   register_restart_handler(>restart);
+
dev_info(>dev, "registration successful\n");
return 0;
 
@@ -253,6 +271,7 @@ static int sp805_wdt_remove(struct amba_device *adev)
 {
struct sp805_wdt *wdt = amba_get_drvdata(adev);
 
+   unregister_restart_handler(>restart);
watchdog_unregister_device(>wdd);
watchdog_set_drvdata(>wdd, NULL);
 
-- 
2.14.1



[PATCH] watchdog: sp805: add restart handler

2018-04-30 Thread Jongsung Kim
Add restart handler for SP805 watchdog so that the driver can be
used to reboot the system.

Signed-off-by: Jongsung Kim 
---
 drivers/watchdog/sp805_wdt.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 01d816251302..01f7b6c29f92 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,6 +68,7 @@ struct sp805_wdt {
struct clk  *clk;
struct amba_device  *adev;
unsigned intload_val;
+   struct notifier_block   restart;
 };
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -200,6 +202,18 @@ static const struct watchdog_ops wdt_ops = {
.get_timeleft   = wdt_timeleft,
 };
 
+static int
+wdt_restart(struct notifier_block *this, unsigned long mode, void *cmd)
+{
+   struct sp805_wdt *wdt = container_of(this, struct sp805_wdt, restart);
+
+   writel_relaxed(0, wdt->base + WDTCONTROL);
+   writel_relaxed(0, wdt->base + WDTLOAD);
+   writel_relaxed(INT_ENABLE | RESET_ENABLE, wdt->base + WDTCONTROL);
+
+   return 0;
+}
+
 static int
 sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 {
@@ -241,6 +255,10 @@ sp805_wdt_probe(struct amba_device *adev, const struct 
amba_id *id)
}
amba_set_drvdata(adev, wdt);
 
+   wdt->restart.notifier_call = wdt_restart;
+   wdt->restart.priority = 128;
+   register_restart_handler(>restart);
+
dev_info(>dev, "registration successful\n");
return 0;
 
@@ -253,6 +271,7 @@ static int sp805_wdt_remove(struct amba_device *adev)
 {
struct sp805_wdt *wdt = amba_get_drvdata(adev);
 
+   unregister_restart_handler(>restart);
watchdog_unregister_device(>wdd);
watchdog_set_drvdata(>wdd, NULL);
 
-- 
2.14.1



Re: [PATCH] arm: module-plts: improve algorithm for counting PLTs

2016-08-17 Thread Jongsung Kim
Hi Ard,

On 2016년 08월 16일 23:39, Ard Biesheuvel wrote:
> (+ Dave)
>
> Hello Jongsung,
>
> On 16 August 2016 at 14:55, Jongsung Kim <neidhard@lge.com> wrote:
>> Current count_plts() uses O(n^2) algorithm for counting distinct
>> PLTs. It's good and fast enough when handling relatively small
>> number of relocs. But the time for counting grows so fast by its
>> nature. A Cortex-A53 operating at 1GHz takes about 10 seconds to
>> count 4,819 distinct PLTs from 257,394 relocs. It can be serious
>> for embedded systems those usually want to boot fast.
> If I take the largest module I can find in my multi_v7_defconfig build, I get
>
> $ readelf -r ./net/mac80211/mac80211.ko |wc -l
> 7984
> $ readelf -r ./net/mac80211/mac80211.ko |grep -E JUMP\|CALL |wc -l
> 3675
>
> Where does the figure 257,394 originate from?
We have a relatively large(~12MB) ko that contains kernel drivers to
support an LGE DTV SoC in development:

$ arm-linux-readelf -r kdrv_lg1313.ko | wc -l
280135
$ arm-linux-readelf -r kdrv_lg1313.ko | grep -E JUMP\|CALL | wc -l
62045

Looks still growing.
>
>> This patch introduces faster O(n) algorithm for counting unique
>> PLTs using hash-table. The following table compares the time (in
>> usecs) for counting distinct PLTs from relocs (using Cortex-A53
>> @1GHz mentioned above):
>>
>> --
>>   relocsPLTs  O(n^2) O(n)
>> --
>>   15   1   1   27
>>   30   6   1   29
>>   60  14   5   31
>>  120  26  15   32
>>  240  47  51   36
>>  480  88 216   50
>>  960 125 560   67
>>1,920 191   1,476  106
>>3,840 253   5,731  179
>>7,680 431  21,226  347
>>   15,360 637  88,211  698
>>   30,720   1,291 331,6261,369
>>   61,440   1,902 803,9642,917
>>  122,880   3,320   4,129,4396,428
>>  245,760   4,646   8,837,064   13,024
>> ==
>>
>> The time increases near-linearly, and the time to handling same
>> 257,394 relocs is reduced to < 20msec from 10 seconds. (< 0.2%)
>>
>> With very small number of PLTs, O(n^2) counting is still faster
>> than O(n) counting, because O(n) counting needs additional O(n)
>> memory space allocation. In these cases, however, the difference
>> looks very short and negligible.
>>
>> This patch does not replaces original O(n^2) counting algorithm
>> with introduced O(n) algorithm, to use it as fall-back algorithm
>> when required memory allocation fails.
>>
> I think there are other optimizations that are much simpler that we
> could look into first. For instance, PLT entries can only be used for
> call and jump relocations that refer to SHN_UNDEF symbols: this is a
> rather fundamental restriction, since the PLT itself must be in range
> for these call and jump instructions. If the module grows so big that
> PLT entries are required for jumps inside the same module, we can no
> longer guarantee that the PLT can be located close enough.
>
> I quickly tested this with the module above:
> Before:
>
> # insmod cfg80211.ko
> [   45.981587] Allocating 238 PLT entries for 3632 external
> jumps/calls (out of 3632 relocations)
> [   45.981967] Allocating 4 PLT entries for 10 external jumps/calls
> (out of 10 relocations)
> [   45.982386] Allocating 19 PLT entries for 37 external jumps/calls
> (out of 37 relocations)
> [   45.982895] Allocating 7 PLT entries for 11 external jumps/calls
> (out of 11 relocations)
> [   45.983409] Allocating 4 PLT entries for 16 external jumps/calls
> (out of 16 relocations)
>
> # insmod mac80211.ko
> [   52.028863] Allocating 545 PLT entries for 5762 external
> jumps/calls (out of 5762 relocations)
> [   52.029207] Allocating 8 PLT entries for 16 external jumps/calls
> (out of 16 relocations)
> [   52.029431] Allocating 4 PLT entries for 4 external jumps/calls
> (out of 4 relocations)
> [   52.029676] Allocating 39 PLT entries for 107 external jumps/calls
> (out of 107 relocations)
>
> (i.e., without the optimization, all jumps and calls are identified as
> potentially external)
>
> After:
>
> # insmod cfg80211.ko
> [   47.685451] Allocating 111 PLT entries for 2097 external
> jumps/calls (out of 3632 relocations)
> [   47.686016] Allocating 3 PLT entri

Re: [PATCH] arm: module-plts: improve algorithm for counting PLTs

2016-08-17 Thread Jongsung Kim
Hi Ard,

On 2016년 08월 16일 23:39, Ard Biesheuvel wrote:
> (+ Dave)
>
> Hello Jongsung,
>
> On 16 August 2016 at 14:55, Jongsung Kim  wrote:
>> Current count_plts() uses O(n^2) algorithm for counting distinct
>> PLTs. It's good and fast enough when handling relatively small
>> number of relocs. But the time for counting grows so fast by its
>> nature. A Cortex-A53 operating at 1GHz takes about 10 seconds to
>> count 4,819 distinct PLTs from 257,394 relocs. It can be serious
>> for embedded systems those usually want to boot fast.
> If I take the largest module I can find in my multi_v7_defconfig build, I get
>
> $ readelf -r ./net/mac80211/mac80211.ko |wc -l
> 7984
> $ readelf -r ./net/mac80211/mac80211.ko |grep -E JUMP\|CALL |wc -l
> 3675
>
> Where does the figure 257,394 originate from?
We have a relatively large(~12MB) ko that contains kernel drivers to
support an LGE DTV SoC in development:

$ arm-linux-readelf -r kdrv_lg1313.ko | wc -l
280135
$ arm-linux-readelf -r kdrv_lg1313.ko | grep -E JUMP\|CALL | wc -l
62045

Looks still growing.
>
>> This patch introduces faster O(n) algorithm for counting unique
>> PLTs using hash-table. The following table compares the time (in
>> usecs) for counting distinct PLTs from relocs (using Cortex-A53
>> @1GHz mentioned above):
>>
>> --
>>   relocsPLTs  O(n^2) O(n)
>> --
>>   15   1   1   27
>>   30   6   1   29
>>   60  14   5   31
>>  120  26  15   32
>>  240  47  51   36
>>  480  88 216   50
>>  960 125 560   67
>>1,920 191   1,476  106
>>3,840 253   5,731  179
>>7,680 431  21,226  347
>>   15,360 637  88,211  698
>>   30,720   1,291 331,6261,369
>>   61,440   1,902 803,9642,917
>>  122,880   3,320   4,129,4396,428
>>  245,760   4,646   8,837,064   13,024
>> ==
>>
>> The time increases near-linearly, and the time to handling same
>> 257,394 relocs is reduced to < 20msec from 10 seconds. (< 0.2%)
>>
>> With very small number of PLTs, O(n^2) counting is still faster
>> than O(n) counting, because O(n) counting needs additional O(n)
>> memory space allocation. In these cases, however, the difference
>> looks very short and negligible.
>>
>> This patch does not replaces original O(n^2) counting algorithm
>> with introduced O(n) algorithm, to use it as fall-back algorithm
>> when required memory allocation fails.
>>
> I think there are other optimizations that are much simpler that we
> could look into first. For instance, PLT entries can only be used for
> call and jump relocations that refer to SHN_UNDEF symbols: this is a
> rather fundamental restriction, since the PLT itself must be in range
> for these call and jump instructions. If the module grows so big that
> PLT entries are required for jumps inside the same module, we can no
> longer guarantee that the PLT can be located close enough.
>
> I quickly tested this with the module above:
> Before:
>
> # insmod cfg80211.ko
> [   45.981587] Allocating 238 PLT entries for 3632 external
> jumps/calls (out of 3632 relocations)
> [   45.981967] Allocating 4 PLT entries for 10 external jumps/calls
> (out of 10 relocations)
> [   45.982386] Allocating 19 PLT entries for 37 external jumps/calls
> (out of 37 relocations)
> [   45.982895] Allocating 7 PLT entries for 11 external jumps/calls
> (out of 11 relocations)
> [   45.983409] Allocating 4 PLT entries for 16 external jumps/calls
> (out of 16 relocations)
>
> # insmod mac80211.ko
> [   52.028863] Allocating 545 PLT entries for 5762 external
> jumps/calls (out of 5762 relocations)
> [   52.029207] Allocating 8 PLT entries for 16 external jumps/calls
> (out of 16 relocations)
> [   52.029431] Allocating 4 PLT entries for 4 external jumps/calls
> (out of 4 relocations)
> [   52.029676] Allocating 39 PLT entries for 107 external jumps/calls
> (out of 107 relocations)
>
> (i.e., without the optimization, all jumps and calls are identified as
> potentially external)
>
> After:
>
> # insmod cfg80211.ko
> [   47.685451] Allocating 111 PLT entries for 2097 external
> jumps/calls (out of 3632 relocations)
> [   47.686016] Allocating 3 PLT entries for 5 external jumps/c

[PATCH] arm: module-plts: improve algorithm for counting PLTs

2016-08-16 Thread Jongsung Kim
Current count_plts() uses O(n^2) algorithm for counting distinct
PLTs. It's good and fast enough when handling relatively small
number of relocs. But the time for counting grows so fast by its
nature. A Cortex-A53 operating at 1GHz takes about 10 seconds to
count 4,819 distinct PLTs from 257,394 relocs. It can be serious
for embedded systems those usually want to boot fast.

This patch introduces faster O(n) algorithm for counting unique
PLTs using hash-table. The following table compares the time (in
usecs) for counting distinct PLTs from relocs (using Cortex-A53
@1GHz mentioned above):

--
  relocsPLTs  O(n^2) O(n)
--
  15   1   1   27
  30   6   1   29
  60  14   5   31
 120  26  15   32
 240  47  51   36
 480  88 216   50
 960 125 560   67
   1,920 191   1,476  106
   3,840 253   5,731  179
   7,680 431  21,226  347
  15,360 637  88,211  698
  30,720   1,291 331,6261,369
  61,440   1,902 803,9642,917
 122,880   3,320   4,129,4396,428
 245,760   4,646   8,837,064   13,024
==

The time increases near-linearly, and the time to handling same
257,394 relocs is reduced to < 20msec from 10 seconds. (< 0.2%)

With very small number of PLTs, O(n^2) counting is still faster
than O(n) counting, because O(n) counting needs additional O(n)
memory space allocation. In these cases, however, the difference
looks very short and negligible.

This patch does not replaces original O(n^2) counting algorithm
with introduced O(n) algorithm, to use it as fall-back algorithm
when required memory allocation fails.

Reported-by: Chanho Min <chanho@lge.com>
Suggested-by: Youngho Shin <youngho.s...@lge.com>
Signed-off-by: Jongsung Kim <neidhard@lge.com>
Reviewed-by: Namhyung Kim <namhyung@lge.com>
---
 arch/arm/kernel/module-plts.c | 111 +-
 1 file changed, 110 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c
index 0c7efc3..dae7459 100644
--- a/arch/arm/kernel/module-plts.c
+++ b/arch/arm/kernel/module-plts.c
@@ -9,6 +9,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -25,11 +27,26 @@
(PLT_ENT_STRIDE - 8))
 #endif
 
+#define PLT_HASH_SHIFT 10
+#define PLT_HASH_SIZE  (1 << PLT_HASH_SHIFT)
+#define PLT_HASH_MASK  (PLT_HASH_SIZE - 1)
+
 struct plt_entries {
u32 ldr[PLT_ENT_COUNT];
u32 lit[PLT_ENT_COUNT];
 };
 
+struct plt_hash_entry {
+   struct plt_hash_entry *next;
+   Elf32_Rel const *plt;
+};
+
+struct plt_hash_table {
+   struct plt_hash_entry *table[PLT_HASH_SIZE];
+   size_t used;
+   struct plt_hash_entry entry[0];
+};
+
 static bool in_init(const struct module *mod, u32 addr)
 {
return addr - (u32)mod->init_layout.base < mod->init_layout.size;
@@ -100,7 +117,7 @@ static int duplicate_rel(Elf32_Addr base, const Elf32_Rel 
*rel, int num,
 }
 
 /* Count how many PLT entries we may need */
-static unsigned int count_plts(Elf32_Addr base, const Elf32_Rel *rel, int num)
+static unsigned int _count_plts(Elf32_Addr base, const Elf32_Rel *rel, int num)
 {
unsigned int ret = 0;
int i;
@@ -129,6 +146,98 @@ static unsigned int count_plts(Elf32_Addr base, const 
Elf32_Rel *rel, int num)
return ret;
 }
 
+static unsigned int hash_plt(Elf32_Rel const *plt, Elf32_Addr base, u32 mask)
+{
+   u32 const *loc = (u32 *)(base + plt->r_offset);
+   u32 hash = (plt->r_info >> 8) ^ (*loc & mask);
+   return hash & PLT_HASH_MASK;
+}
+
+static bool
+same_plts(Elf32_Rel const *a, Elf32_Rel const *b, Elf32_Addr base, u32 mask)
+{
+   u32 const *loc1;
+   u32 const *loc2;
+
+   if (a->r_info != b->r_info)
+   return false;
+
+   loc1 = (u32 *)(base + a->r_offset);
+   loc2 = (u32 *)(base + b->r_offset);
+
+   return ((*loc1 ^ *loc2) & mask) == 0;
+}
+
+static int hash_insert_plt(struct plt_hash_table *table, Elf32_Rel const *plt,
+  Elf32_Addr base, u32 mask)
+{
+   unsigned int hash = hash_plt(plt, base, mask);
+   struct plt_hash_entry *entry;
+
+   for (entry = table->table[hash]; entry; entry = entry->next)
+   if (same_plts(entry->plt, plt, base, mask))
+   return 0;
+
+   entry = >entry[table->used++];
+   entry->next = table->table[hash];
+   entry->plt

[PATCH] arm: module-plts: improve algorithm for counting PLTs

2016-08-16 Thread Jongsung Kim
Current count_plts() uses O(n^2) algorithm for counting distinct
PLTs. It's good and fast enough when handling relatively small
number of relocs. But the time for counting grows so fast by its
nature. A Cortex-A53 operating at 1GHz takes about 10 seconds to
count 4,819 distinct PLTs from 257,394 relocs. It can be serious
for embedded systems those usually want to boot fast.

This patch introduces faster O(n) algorithm for counting unique
PLTs using hash-table. The following table compares the time (in
usecs) for counting distinct PLTs from relocs (using Cortex-A53
@1GHz mentioned above):

--
  relocsPLTs  O(n^2) O(n)
--
  15   1   1   27
  30   6   1   29
  60  14   5   31
 120  26  15   32
 240  47  51   36
 480  88 216   50
 960 125 560   67
   1,920 191   1,476  106
   3,840 253   5,731  179
   7,680 431  21,226  347
  15,360 637  88,211  698
  30,720   1,291 331,6261,369
  61,440   1,902 803,9642,917
 122,880   3,320   4,129,4396,428
 245,760   4,646   8,837,064   13,024
==

The time increases near-linearly, and the time to handling same
257,394 relocs is reduced to < 20msec from 10 seconds. (< 0.2%)

With very small number of PLTs, O(n^2) counting is still faster
than O(n) counting, because O(n) counting needs additional O(n)
memory space allocation. In these cases, however, the difference
looks very short and negligible.

This patch does not replaces original O(n^2) counting algorithm
with introduced O(n) algorithm, to use it as fall-back algorithm
when required memory allocation fails.

Reported-by: Chanho Min 
Suggested-by: Youngho Shin 
Signed-off-by: Jongsung Kim 
Reviewed-by: Namhyung Kim 
---
 arch/arm/kernel/module-plts.c | 111 +-
 1 file changed, 110 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c
index 0c7efc3..dae7459 100644
--- a/arch/arm/kernel/module-plts.c
+++ b/arch/arm/kernel/module-plts.c
@@ -9,6 +9,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -25,11 +27,26 @@
(PLT_ENT_STRIDE - 8))
 #endif
 
+#define PLT_HASH_SHIFT 10
+#define PLT_HASH_SIZE  (1 << PLT_HASH_SHIFT)
+#define PLT_HASH_MASK  (PLT_HASH_SIZE - 1)
+
 struct plt_entries {
u32 ldr[PLT_ENT_COUNT];
u32 lit[PLT_ENT_COUNT];
 };
 
+struct plt_hash_entry {
+   struct plt_hash_entry *next;
+   Elf32_Rel const *plt;
+};
+
+struct plt_hash_table {
+   struct plt_hash_entry *table[PLT_HASH_SIZE];
+   size_t used;
+   struct plt_hash_entry entry[0];
+};
+
 static bool in_init(const struct module *mod, u32 addr)
 {
return addr - (u32)mod->init_layout.base < mod->init_layout.size;
@@ -100,7 +117,7 @@ static int duplicate_rel(Elf32_Addr base, const Elf32_Rel 
*rel, int num,
 }
 
 /* Count how many PLT entries we may need */
-static unsigned int count_plts(Elf32_Addr base, const Elf32_Rel *rel, int num)
+static unsigned int _count_plts(Elf32_Addr base, const Elf32_Rel *rel, int num)
 {
unsigned int ret = 0;
int i;
@@ -129,6 +146,98 @@ static unsigned int count_plts(Elf32_Addr base, const 
Elf32_Rel *rel, int num)
return ret;
 }
 
+static unsigned int hash_plt(Elf32_Rel const *plt, Elf32_Addr base, u32 mask)
+{
+   u32 const *loc = (u32 *)(base + plt->r_offset);
+   u32 hash = (plt->r_info >> 8) ^ (*loc & mask);
+   return hash & PLT_HASH_MASK;
+}
+
+static bool
+same_plts(Elf32_Rel const *a, Elf32_Rel const *b, Elf32_Addr base, u32 mask)
+{
+   u32 const *loc1;
+   u32 const *loc2;
+
+   if (a->r_info != b->r_info)
+   return false;
+
+   loc1 = (u32 *)(base + a->r_offset);
+   loc2 = (u32 *)(base + b->r_offset);
+
+   return ((*loc1 ^ *loc2) & mask) == 0;
+}
+
+static int hash_insert_plt(struct plt_hash_table *table, Elf32_Rel const *plt,
+  Elf32_Addr base, u32 mask)
+{
+   unsigned int hash = hash_plt(plt, base, mask);
+   struct plt_hash_entry *entry;
+
+   for (entry = table->table[hash]; entry; entry = entry->next)
+   if (same_plts(entry->plt, plt, base, mask))
+   return 0;
+
+   entry = >entry[table->used++];
+   entry->next = table->table[hash];
+   entry->plt = plt;
+   table->table[hash] = entry;
+
+   return 1;
+}
+
+static size_t count_plts(Elf

Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-07-03 Thread Jongsung Kim
On 2016년 07월 02일 09:20, Stephen Boyd wrote:
> On 06/29, Jongsung Kim wrote:
>> On 2016년 06월 29일 06:18, Michael Turquette wrote:
>>> Quoting Rob Herring (2016-06-28 13:55:18)
>>>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>>>> There is no way to set additional flags for a DT-initialized fixed-
>>>>> factor-clock, and it can be problematic i.e., when the clock rate
>>>>> needs to be changed. [1][2]
>>>>>
>>>>> This patch introduces an optional dt-binding named "clock-flags" to
>>>>> be used for passing any needed flags from dts.
>>>> I don't think we want this in DT. If we did, the flags would need some 
>>>> documentation about what the flags mean.
>>> Flags are specific to Linux implementation, so I agree with Rob. Better
>>> to create a compatible string for your hardware that bakes in the flags.
>> Thank you for your comment, Mike. This conversation starts from lacking 
>> method to set CLK_SET_RATE_PARENT from DT. I understand compatible string 
>> can be a solution. But.. if someone starts talking about lacking method to 
>> set another flag, i.e., CLK_SET_PARENT_GATE, then we'll need another 
>> compatible string list.
>> How do you think about defining possible required subset of the flags and 
>> using some more neutral flag-names acceptable in DT?
> Do you actually have an IC on the board that is doing some fixed
> factor calculation? Or is this a clk driver design where we are
> listing out each piece of an SoC's clk controller in DT?
>
The SoC has several PLLs of identical design, and one of them is divided
to half and used for CPUs. The fixed-factor-clock represents the divider.



Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-07-03 Thread Jongsung Kim
On 2016년 07월 02일 09:20, Stephen Boyd wrote:
> On 06/29, Jongsung Kim wrote:
>> On 2016년 06월 29일 06:18, Michael Turquette wrote:
>>> Quoting Rob Herring (2016-06-28 13:55:18)
>>>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>>>> There is no way to set additional flags for a DT-initialized fixed-
>>>>> factor-clock, and it can be problematic i.e., when the clock rate
>>>>> needs to be changed. [1][2]
>>>>>
>>>>> This patch introduces an optional dt-binding named "clock-flags" to
>>>>> be used for passing any needed flags from dts.
>>>> I don't think we want this in DT. If we did, the flags would need some 
>>>> documentation about what the flags mean.
>>> Flags are specific to Linux implementation, so I agree with Rob. Better
>>> to create a compatible string for your hardware that bakes in the flags.
>> Thank you for your comment, Mike. This conversation starts from lacking 
>> method to set CLK_SET_RATE_PARENT from DT. I understand compatible string 
>> can be a solution. But.. if someone starts talking about lacking method to 
>> set another flag, i.e., CLK_SET_PARENT_GATE, then we'll need another 
>> compatible string list.
>> How do you think about defining possible required subset of the flags and 
>> using some more neutral flag-names acceptable in DT?
> Do you actually have an IC on the board that is doing some fixed
> factor calculation? Or is this a clk driver design where we are
> listing out each piece of an SoC's clk controller in DT?
>
The SoC has several PLLs of identical design, and one of them is divided
to half and used for CPUs. The fixed-factor-clock represents the divider.



Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-29 Thread Jongsung Kim
On 2016년 06월 29일 06:18, Michael Turquette wrote:
> Quoting Rob Herring (2016-06-28 13:55:18)
>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>> There is no way to set additional flags for a DT-initialized fixed-
>>> factor-clock, and it can be problematic i.e., when the clock rate
>>> needs to be changed. [1][2]
>>>
>>> This patch introduces an optional dt-binding named "clock-flags" to
>>> be used for passing any needed flags from dts.
>> I don't think we want this in DT. If we did, the flags would need some 
>> documentation about what the flags mean.
> Flags are specific to Linux implementation, so I agree with Rob. Better
> to create a compatible string for your hardware that bakes in the flags.

Thank you for your comment, Mike. This conversation starts from lacking method 
to set CLK_SET_RATE_PARENT from DT. I understand compatible string can be a 
solution. But.. if someone starts talking about lacking method to set another 
flag, i.e., CLK_SET_PARENT_GATE, then we'll need another compatible string list.
How do you think about defining possible required subset of the flags and using 
some more neutral flag-names acceptable in DT?


Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-29 Thread Jongsung Kim
On 2016년 06월 29일 06:18, Michael Turquette wrote:
> Quoting Rob Herring (2016-06-28 13:55:18)
>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>> There is no way to set additional flags for a DT-initialized fixed-
>>> factor-clock, and it can be problematic i.e., when the clock rate
>>> needs to be changed. [1][2]
>>>
>>> This patch introduces an optional dt-binding named "clock-flags" to
>>> be used for passing any needed flags from dts.
>> I don't think we want this in DT. If we did, the flags would need some 
>> documentation about what the flags mean.
> Flags are specific to Linux implementation, so I agree with Rob. Better
> to create a compatible string for your hardware that bakes in the flags.

Thank you for your comment, Mike. This conversation starts from lacking method 
to set CLK_SET_RATE_PARENT from DT. I understand compatible string can be a 
solution. But.. if someone starts talking about lacking method to set another 
flag, i.e., CLK_SET_PARENT_GATE, then we'll need another compatible string list.
How do you think about defining possible required subset of the flags and using 
some more neutral flag-names acceptable in DT?


Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-28 Thread Jongsung Kim
On 2016년 06월 29일 05:55, Rob Herring wrote:
> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>> There is no way to set additional flags for a DT-initialized fixed-
>> factor-clock, and it can be problematic i.e., when the clock rate
>> needs to be changed. [1][2]
>>
>> This patch introduces an optional dt-binding named "clock-flags" to
>> be used for passing any needed flags from dts.
> I don't think we want this in DT. If we did, the flags would need some 
> documentation about what the flags mean.
Thanks for your comment. It looks not that big deal to provide a little 
documentation..? Some of the flags looks safe to be removed. Thinking of only 
fixed-factor-clock, most of them can be removed.


Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-28 Thread Jongsung Kim
On 2016년 06월 29일 05:55, Rob Herring wrote:
> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>> There is no way to set additional flags for a DT-initialized fixed-
>> factor-clock, and it can be problematic i.e., when the clock rate
>> needs to be changed. [1][2]
>>
>> This patch introduces an optional dt-binding named "clock-flags" to
>> be used for passing any needed flags from dts.
> I don't think we want this in DT. If we did, the flags would need some 
> documentation about what the flags mean.
Thanks for your comment. It looks not that big deal to provide a little 
documentation..? Some of the flags looks safe to be removed. Thinking of only 
fixed-factor-clock, most of them can be removed.


[PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-23 Thread Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]

This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.

[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025

Changes since v1:
 - fix possible build failure when using gcc-5 or gcc-6

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Cc: Maxime Ripard <maxime.rip...@free-electrons.com>
Cc: Mike Turquette <mturque...@baylibre.com>
Cc: Stephen Boyd <sb...@codeaurora.org>
---
 .../bindings/clock/fixed-factor-clock.txt  |  4 
 drivers/clk/clk-fixed-factor.c |  4 +++-
 include/dt-bindings/clk/clk.h  | 22 ++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clk/clk.h

diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt 
b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
 
 Optional properties:
 - clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
 
 Example:
+   #include 
+
clock {
compatible = "fixed-factor-clock";
clocks = <>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+   clock-flags = ;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..e626cad 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+   u32 flags = 0;
u32 div, mult;
 
if (of_property_read_u32(node, "clock-div", )) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
 
of_property_read_string(node, "clock-output-names", _name);
parent_name = of_clk_get_parent_name(node, 0);
+   of_property_read_u32(node, "clock-flags", );
 
-   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr)(1UL << (nr))
+
+#define CLK_SET_RATE_GATE  BIT(0)
+#define CLK_SET_PARENT_GATEBIT(1)
+#define CLK_SET_RATE_PARENTBIT(2)
+#define CLK_IGNORE_UNUSED  BIT(3)
+#define CLK_IS_BASIC   BIT(5)
+#define CLK_GET_RATE_NOCACHE   BIT(6)
+#define CLK_SET_RATE_NO_REPARENT   BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE   BIT(8)
+#define CLK_RECALC_NEW_RATES   BIT(9)
+#define CLK_SET_RATE_UNGATEBIT(10)
+#define CLK_IS_CRITICALBIT(11)
+
+#endif
-- 
2.7.4



[PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-23 Thread Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]

This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.

[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025

Changes since v1:
 - fix possible build failure when using gcc-5 or gcc-6

Signed-off-by: Jongsung Kim 
Cc: Maxime Ripard 
Cc: Mike Turquette 
Cc: Stephen Boyd 
---
 .../bindings/clock/fixed-factor-clock.txt  |  4 
 drivers/clk/clk-fixed-factor.c |  4 +++-
 include/dt-bindings/clk/clk.h  | 22 ++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clk/clk.h

diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt 
b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
 
 Optional properties:
 - clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
 
 Example:
+   #include 
+
clock {
compatible = "fixed-factor-clock";
clocks = <>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+   clock-flags = ;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..e626cad 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+   u32 flags = 0;
u32 div, mult;
 
if (of_property_read_u32(node, "clock-div", )) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
 
of_property_read_string(node, "clock-output-names", _name);
parent_name = of_clk_get_parent_name(node, 0);
+   of_property_read_u32(node, "clock-flags", );
 
-   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr)(1UL << (nr))
+
+#define CLK_SET_RATE_GATE  BIT(0)
+#define CLK_SET_PARENT_GATEBIT(1)
+#define CLK_SET_RATE_PARENTBIT(2)
+#define CLK_IGNORE_UNUSED  BIT(3)
+#define CLK_IS_BASIC   BIT(5)
+#define CLK_GET_RATE_NOCACHE   BIT(6)
+#define CLK_SET_RATE_NO_REPARENT   BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE   BIT(8)
+#define CLK_RECALC_NEW_RATES   BIT(9)
+#define CLK_SET_RATE_UNGATEBIT(10)
+#define CLK_IS_CRITICALBIT(11)
+
+#endif
-- 
2.7.4



[PATCH] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-23 Thread Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]

This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.

[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025

Signed-off-by: Jongsung Kim <neidhard@lge.com>
Cc: Maxime Ripard <maxime.rip...@free-electrons.com>
Cc: Mike Turquette <mturque...@baylibre.com>
Cc: Stephen Boyd <sb...@codeaurora.org>
---
 .../bindings/clock/fixed-factor-clock.txt  |  4 
 drivers/clk/clk-fixed-factor.c |  4 +++-
 include/dt-bindings/clk/clk.h  | 22 ++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clk/clk.h

diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt 
b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
 
 Optional properties:
 - clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
 
 Example:
+   #include 
+
clock {
compatible = "fixed-factor-clock";
clocks = <>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+   clock-flags = ;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..da3cd9c 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+   unsigned long flags = 0;
u32 div, mult;
 
if (of_property_read_u32(node, "clock-div", )) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
 
of_property_read_string(node, "clock-output-names", _name);
parent_name = of_clk_get_parent_name(node, 0);
+   of_property_read_u32(node, "clock-flags", );
 
-   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr)(1UL << (nr))
+
+#define CLK_SET_RATE_GATE  BIT(0)
+#define CLK_SET_PARENT_GATEBIT(1)
+#define CLK_SET_RATE_PARENTBIT(2)
+#define CLK_IGNORE_UNUSED  BIT(3)
+#define CLK_IS_BASIC   BIT(5)
+#define CLK_GET_RATE_NOCACHE   BIT(6)
+#define CLK_SET_RATE_NO_REPARENT   BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE   BIT(8)
+#define CLK_RECALC_NEW_RATES   BIT(9)
+#define CLK_SET_RATE_UNGATEBIT(10)
+#define CLK_IS_CRITICALBIT(11)
+
+#endif
-- 
2.7.4



[PATCH] clk: fixed-factor: add optional dt-binding clock-flags

2016-06-23 Thread Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]

This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.

[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025

Signed-off-by: Jongsung Kim 
Cc: Maxime Ripard 
Cc: Mike Turquette 
Cc: Stephen Boyd 
---
 .../bindings/clock/fixed-factor-clock.txt  |  4 
 drivers/clk/clk-fixed-factor.c |  4 +++-
 include/dt-bindings/clk/clk.h  | 22 ++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/clk/clk.h

diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt 
b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
 
 Optional properties:
 - clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
 
 Example:
+   #include 
+
clock {
compatible = "fixed-factor-clock";
clocks = <>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+   clock-flags = ;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..da3cd9c 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+   unsigned long flags = 0;
u32 div, mult;
 
if (of_property_read_u32(node, "clock-div", )) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node 
*node)
 
of_property_read_string(node, "clock-output-names", _name);
parent_name = of_clk_get_parent_name(node, 0);
+   of_property_read_u32(node, "clock-flags", );
 
-   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+   clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr)(1UL << (nr))
+
+#define CLK_SET_RATE_GATE  BIT(0)
+#define CLK_SET_PARENT_GATEBIT(1)
+#define CLK_SET_RATE_PARENTBIT(2)
+#define CLK_IGNORE_UNUSED  BIT(3)
+#define CLK_IS_BASIC   BIT(5)
+#define CLK_GET_RATE_NOCACHE   BIT(6)
+#define CLK_SET_RATE_NO_REPARENT   BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE   BIT(8)
+#define CLK_RECALC_NEW_RATES   BIT(9)
+#define CLK_SET_RATE_UNGATEBIT(10)
+#define CLK_IS_CRITICALBIT(11)
+
+#endif
-- 
2.7.4



[RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

2016-06-13 Thread Jongsung Kim
Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
returns the current frequency. A fixed-factor-clock initialzed via
of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
set. It can be problematic when the parent of a fixed-factor-clock
is rate-controllable clk, because the rounding can't be propagated
to parent, the rounded target frequency is always the current, and
finally the frequency can't be changed.

This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
clocks, from clk_register_fixed_factor(), and removes checking the
flag from clk_factor_round_rate().

Signed-off-by: Jongsung Kim <neidhard@lge.com>
---
 drivers/clk/clk-fixed-factor.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..9568306 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -38,13 +38,10 @@ static long clk_factor_round_rate(struct clk_hw *hw, 
unsigned long rate,
unsigned long *prate)
 {
struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+   unsigned long best_parent;
 
-   if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
-   unsigned long best_parent;
-
-   best_parent = (rate / fix->mult) * fix->div;
-   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
-   }
+   best_parent = (rate / fix->mult) * fix->div;
+   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
 
return (*prate / fix->div) * fix->mult;
 }
@@ -88,7 +85,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device 
*dev,
 
init.name = name;
init.ops = _fixed_factor_ops;
-   init.flags = flags | CLK_IS_BASIC;
+   init.flags = flags | CLK_IS_BASIC | CLK_SET_RATE_PARENT;
init.parent_names = _name;
init.num_parents = 1;
 
-- 
2.7.4



[RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

2016-06-13 Thread Jongsung Kim
Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
returns the current frequency. A fixed-factor-clock initialzed via
of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
set. It can be problematic when the parent of a fixed-factor-clock
is rate-controllable clk, because the rounding can't be propagated
to parent, the rounded target frequency is always the current, and
finally the frequency can't be changed.

This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
clocks, from clk_register_fixed_factor(), and removes checking the
flag from clk_factor_round_rate().

Signed-off-by: Jongsung Kim 
---
 drivers/clk/clk-fixed-factor.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..9568306 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -38,13 +38,10 @@ static long clk_factor_round_rate(struct clk_hw *hw, 
unsigned long rate,
unsigned long *prate)
 {
struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+   unsigned long best_parent;
 
-   if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
-   unsigned long best_parent;
-
-   best_parent = (rate / fix->mult) * fix->div;
-   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
-   }
+   best_parent = (rate / fix->mult) * fix->div;
+   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
 
return (*prate / fix->div) * fix->mult;
 }
@@ -88,7 +85,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device 
*dev,
 
init.name = name;
init.ops = _fixed_factor_ops;
-   init.flags = flags | CLK_IS_BASIC;
+   init.flags = flags | CLK_IS_BASIC | CLK_SET_RATE_PARENT;
init.parent_names = _name;
init.num_parents = 1;
 
-- 
2.7.4



[PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

2016-06-02 Thread Jongsung Kim
Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
returns the current frequency. A fixed-factor-clock initialzed via
of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
set. It can be problematic when the parent of a fixed-factor-clock
is rate-controllable clk, because the rounding can't be propagated
to parent, the rounded target frequency is always the current, and
finally the frequency can't be changed.

This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
clocks, from clk_register_fixed_factor(), and removes checking the
flag from clk_factor_round_rate().

Signed-off-by: Jongsung Kim <neidhard@lge.com>
---
 drivers/clk/clk-fixed-factor.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..9b5df847f 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -38,13 +38,10 @@ static long clk_factor_round_rate(struct clk_hw *hw, 
unsigned long rate,
unsigned long *prate)
 {
struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+   unsigned long best_parent;
 
-   if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
-   unsigned long best_parent;
-
-   best_parent = (rate / fix->mult) * fix->div;
-   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
-   }
+   best_parent  = (rate / fix->mult) * fix->div;
+   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
 
return (*prate / fix->div) * fix->mult;
 }
@@ -88,7 +85,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device 
*dev,
 
init.name = name;
init.ops = _fixed_factor_ops;
-   init.flags = flags | CLK_IS_BASIC;
+   init.flags = flags | CLK_IS_BASIC | CLK_SET_RATE_PARENT;
init.parent_names = _name;
init.num_parents = 1;
 
-- 
2.7.4



[PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

2016-06-02 Thread Jongsung Kim
Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
returns the current frequency. A fixed-factor-clock initialzed via
of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
set. It can be problematic when the parent of a fixed-factor-clock
is rate-controllable clk, because the rounding can't be propagated
to parent, the rounded target frequency is always the current, and
finally the frequency can't be changed.

This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
clocks, from clk_register_fixed_factor(), and removes checking the
flag from clk_factor_round_rate().

Signed-off-by: Jongsung Kim 
---
 drivers/clk/clk-fixed-factor.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..9b5df847f 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -38,13 +38,10 @@ static long clk_factor_round_rate(struct clk_hw *hw, 
unsigned long rate,
unsigned long *prate)
 {
struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+   unsigned long best_parent;
 
-   if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
-   unsigned long best_parent;
-
-   best_parent = (rate / fix->mult) * fix->div;
-   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
-   }
+   best_parent  = (rate / fix->mult) * fix->div;
+   *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
 
return (*prate / fix->div) * fix->mult;
 }
@@ -88,7 +85,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device 
*dev,
 
init.name = name;
init.ops = _fixed_factor_ops;
-   init.flags = flags | CLK_IS_BASIC;
+   init.flags = flags | CLK_IS_BASIC | CLK_SET_RATE_PARENT;
init.parent_names = _name;
init.num_parents = 1;
 
-- 
2.7.4



Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-12 Thread Jongsung Kim
On 10/12/2015 06:27 PM, Arnd Bergmann wrote:
> How about changing the zcomp code to pass the page pointer instead of the 
> kernel space pointer? That would avoid having to do the kmap_atomic, which 
> can itself be expensive on 32-bit machines and should not be needed here if 
> you have a HW DMA engine doing the compression. Arnd 

Mainline zram uses lzo / lz4 library functions as backend. Using kmap_atomic 
and passing address look reasonable.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-12 Thread Jongsung Kim
On 10/12/2015 06:27 PM, Arnd Bergmann wrote:
> How about changing the zcomp code to pass the page pointer instead of the 
> kernel space pointer? That would avoid having to do the kmap_atomic, which 
> can itself be expensive on 32-bit machines and should not be needed here if 
> you have a HW DMA engine doing the compression. Arnd 

Mainline zram uses lzo / lz4 library functions as backend. Using kmap_atomic 
and passing address look reasonable.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-11 Thread Jongsung Kim
We tried to utilize a HW compressor as a zram backend. Current zram uses 
kmap_atomic to map a page, and the HW DMAes. So we needed to use 
kmap_atomic_to_page to get the page to be dma-mapped.


On 10/07/2015 06:01 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 07, 2015 at 12:55:08PM +0900, Jongsung Kim wrote:
>> Recently, we made a driver utilizing kmap_atomic_to_page. Of course,
>> it's not mainlined. People may be using it outside mainline just like us.
> Since kmap_atomic() mappings are supposed to be short-lived, why do you
> need it in your driver?  Don't you already have the struct page pointer
> when setting up the kmap_atomic() mapping?
>
> It is invalid to setup a mapping, and leave it setup across any context
> switching or similar.
>
> Also, kmap_atomic_to_page() is not exported to modules, so you can only
> use it when built-in.
>
>> vmalloc has vmalloc_to_page, pkmap has kmap_to page, and fixmap has
>> kmap_atomic_to_page. Then.. how about letting virt_to_page do them all?
> No.  virt_to_page() is defined to only work on the lowmem mapping, and
> that's not going to change.
>
> Please show the outline of your code making use of this function so we
> can better understand your use case.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-11 Thread Jongsung Kim
We tried to utilize a HW compressor as a zram backend. Current zram uses 
kmap_atomic to map a page, and the HW DMAes. So we needed to use 
kmap_atomic_to_page to get the page to be dma-mapped.


On 10/07/2015 06:01 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 07, 2015 at 12:55:08PM +0900, Jongsung Kim wrote:
>> Recently, we made a driver utilizing kmap_atomic_to_page. Of course,
>> it's not mainlined. People may be using it outside mainline just like us.
> Since kmap_atomic() mappings are supposed to be short-lived, why do you
> need it in your driver?  Don't you already have the struct page pointer
> when setting up the kmap_atomic() mapping?
>
> It is invalid to setup a mapping, and leave it setup across any context
> switching or similar.
>
> Also, kmap_atomic_to_page() is not exported to modules, so you can only
> use it when built-in.
>
>> vmalloc has vmalloc_to_page, pkmap has kmap_to page, and fixmap has
>> kmap_atomic_to_page. Then.. how about letting virt_to_page do them all?
> No.  virt_to_page() is defined to only work on the lowmem mapping, and
> that's not going to change.
>
> Please show the outline of your code making use of this function so we
> can better understand your use case.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-06 Thread Jongsung Kim
Recently, we made a driver utilizing kmap_atomic_to_page. Of course, it's not 
mainlined. People may be using it outside mainline just like us.

vmalloc has vmalloc_to_page, pkmap has kmap_to page, and fixmap has 
kmap_atomic_to_page. Then.. how about letting virt_to_page do them all?


On 10/07/2015 10:37 AM, Nicolas Pitre wrote:
> On Tue, 6 Oct 2015, Russell King - ARM Linux wrote:
>
>> On Tue, Oct 06, 2015 at 08:09:33PM +0900, Chanho Min wrote:
>>> Since kmap_atomic returns the pkmap address without a new mapping to
>>> fixmap for the page that is already mapped by kmap, It should be
>>> considered for the pkmap address in kmap_atomic_to_page.
>> What's the reasoning behind this change, given that I can find lots of
>> definitions of kmap_atomic_to_page() in the kernel, but not a single
>> user of this.
>>
>> If there's no users, should we be deleting this code?
> I think commit 5bbeed12bdc3 provides the answer to that question.
>
>
> Nicolas
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM:mm: fix kmap_atomic_to_page

2015-10-06 Thread Jongsung Kim
Recently, we made a driver utilizing kmap_atomic_to_page. Of course, it's not 
mainlined. People may be using it outside mainline just like us.

vmalloc has vmalloc_to_page, pkmap has kmap_to page, and fixmap has 
kmap_atomic_to_page. Then.. how about letting virt_to_page do them all?


On 10/07/2015 10:37 AM, Nicolas Pitre wrote:
> On Tue, 6 Oct 2015, Russell King - ARM Linux wrote:
>
>> On Tue, Oct 06, 2015 at 08:09:33PM +0900, Chanho Min wrote:
>>> Since kmap_atomic returns the pkmap address without a new mapping to
>>> fixmap for the page that is already mapped by kmap, It should be
>>> considered for the pkmap address in kmap_atomic_to_page.
>> What's the reasoning behind this change, given that I can find lots of
>> definitions of kmap_atomic_to_page() in the kernel, but not a single
>> user of this.
>>
>> If there's no users, should we be deleting this code?
> I think commit 5bbeed12bdc3 provides the answer to that question.
>
>
> Nicolas
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-15 Thread Jongsung Kim
On 07/14/2014 07:15 PM, Varka Bhadram wrote:
> 
> I think we can do in this way:
> 
> if (phydev)
> return phy_ethtool_set_wol(phydev, wol);
> else
> return -ENODEV;
> 
> 
> we can save err. What do you say ...?
> 

Thank you for your comment. I just wanted the function to return
at one position. However, I'll think about your point..

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-15 Thread Jongsung Kim
On 07/14/2014 07:49 PM, David Laight wrote:
> From: Varka Bhadram
>> On 07/14/2014 02:32 PM, Jongsung Kim wrote:
>>> This patch enables the ethtool utility to control the WOL function
>>> of the PHY connected to the GEM/MACB. (if supported)
> ...
>>> +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo 
>>> *wol)
>>> +{
>>> +   struct macb *bp = netdev_priv(netdev);
>>> +   struct phy_device *phydev = bp->phy_dev;
>>> +   int err = -ENODEV;
>>> +
>>> +   if (phydev)
>>> +   err = phy_ethtool_set_wol(phydev, wol);
>>> +
>>> +   return err;
>>> +}
>>> +
>>
>> I think we can do in this way:
>>
>> if (phydev)
>>  return phy_ethtool_set_wol(phydev, wol);
>> else
>>  return -ENODEV;
>>
>>
>> we can save err. What do you say ...?
> 
> I would do:
>   if (!phydev)
>   return -ENODEV;
>   return phy_ethtool_set_wol(phydev, wol);
> 
> Although it might even be worth moving the NULL test into the function.
> (sort of depends on style and the number of callers who need to do the test.)
> 

Totally agreed. I'd be better to submit a patch about it first.

>   David
> 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-15 Thread Jongsung Kim
On 07/14/2014 07:49 PM, David Laight wrote:
 From: Varka Bhadram
 On 07/14/2014 02:32 PM, Jongsung Kim wrote:
 This patch enables the ethtool utility to control the WOL function
 of the PHY connected to the GEM/MACB. (if supported)
 ...
 +static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo 
 *wol)
 +{
 +   struct macb *bp = netdev_priv(netdev);
 +   struct phy_device *phydev = bp-phy_dev;
 +   int err = -ENODEV;
 +
 +   if (phydev)
 +   err = phy_ethtool_set_wol(phydev, wol);
 +
 +   return err;
 +}
 +

 I think we can do in this way:

 if (phydev)
  return phy_ethtool_set_wol(phydev, wol);
 else
  return -ENODEV;


 we can save err. What do you say ...?
 
 I would do:
   if (!phydev)
   return -ENODEV;
   return phy_ethtool_set_wol(phydev, wol);
 
 Although it might even be worth moving the NULL test into the function.
 (sort of depends on style and the number of callers who need to do the test.)
 

Totally agreed. I'd be better to submit a patch about it first.

   David
 
 
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-15 Thread Jongsung Kim
On 07/14/2014 07:15 PM, Varka Bhadram wrote:
 
 I think we can do in this way:
 
 if (phydev)
 return phy_ethtool_set_wol(phydev, wol);
 else
 return -ENODEV;
 
 
 we can save err. What do you say ...?
 

Thank you for your comment. I just wanted the function to return
at one position. However, I'll think about your point..

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-14 Thread Jongsung Kim
This patch enables the ethtool utility to control the WOL function
of the PHY connected to the GEM/MACB. (if supported)

Signed-off-by: Jongsung Kim 
---
 drivers/net/ethernet/cadence/macb.c |   26 ++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index e9daa07..7ad8909 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1742,11 +1742,37 @@ static void macb_get_regs(struct net_device *dev, 
struct ethtool_regs *regs,
}
 }
 
+static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo 
*wol)
+{
+   struct macb *bp = netdev_priv(netdev);
+   struct phy_device *phydev = bp->phy_dev;
+
+   wol->supported = 0;
+   wol->wolopts = 0;
+
+   if (phydev)
+   phy_ethtool_get_wol(phydev, wol);
+}
+
+static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct macb *bp = netdev_priv(netdev);
+   struct phy_device *phydev = bp->phy_dev;
+   int err = -ENODEV;
+
+   if (phydev)
+   err = phy_ethtool_set_wol(phydev, wol);
+
+   return err;
+}
+
 const struct ethtool_ops macb_ethtool_ops = {
.get_settings   = macb_get_settings,
.set_settings   = macb_set_settings,
.get_regs_len   = macb_get_regs_len,
.get_regs   = macb_get_regs,
+   .get_wol= macb_get_wol,
+   .set_wol= macb_set_wol,
.get_link   = ethtool_op_get_link,
.get_ts_info= ethtool_op_get_ts_info,
 };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] net: cadence: macb: add support for the WOL

2014-07-14 Thread Jongsung Kim
This patch enables the ethtool utility to control the WOL function
of the PHY connected to the GEM/MACB. (if supported)

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/ethernet/cadence/macb.c |   26 ++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index e9daa07..7ad8909 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1742,11 +1742,37 @@ static void macb_get_regs(struct net_device *dev, 
struct ethtool_regs *regs,
}
 }
 
+static void macb_get_wol(struct net_device *netdev, struct ethtool_wolinfo 
*wol)
+{
+   struct macb *bp = netdev_priv(netdev);
+   struct phy_device *phydev = bp-phy_dev;
+
+   wol-supported = 0;
+   wol-wolopts = 0;
+
+   if (phydev)
+   phy_ethtool_get_wol(phydev, wol);
+}
+
+static int macb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct macb *bp = netdev_priv(netdev);
+   struct phy_device *phydev = bp-phy_dev;
+   int err = -ENODEV;
+
+   if (phydev)
+   err = phy_ethtool_set_wol(phydev, wol);
+
+   return err;
+}
+
 const struct ethtool_ops macb_ethtool_ops = {
.get_settings   = macb_get_settings,
.set_settings   = macb_set_settings,
.get_regs_len   = macb_get_regs_len,
.get_regs   = macb_get_regs,
+   .get_wol= macb_get_wol,
+   .set_wol= macb_set_wol,
.get_link   = ethtool_op_get_link,
.get_ts_info= ethtool_op_get_ts_info,
 };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-18 Thread Jongsung Kim
On 06/17/2014 04:54 PM, Nicolas Ferre wrote:

Hi Nicolas,

> On 17/06/2014 05:39, Jongsung Kim :
>> On 06/17/2014 06:28 AM, Sören Brinkmann wrote:
>>> Shouldn't it be sufficient to replace 'MACB_BIT(RCOMP) with 
>>> 'MACB_RX_INT_FLAGS'
>>> to clear all the RX IRQ flags.
>>
>> I'm afraid not.
>>
>> You know, this driver initially targeted only GEMs configured with 
>> "gem_irq_clear_read."
>> For this implementation of GEM, the ISR is automatically cleared by reading. 
>> The driver
>> was designed to operate with the value read from ISR, not with the ISR 
>> itself.
>>
>> However, there are other GEMs configured without "gem_irq_clear_read," 
>> people like you
>> and I working with. To support them, they insert similar codes conditionally 
>> clearing
>> the ISR here and there. Now they are found at 6 places. Not enough yet. Do 
>> you want to
>> insert another at the end of macb_reset_hw..? Maybe not.
> 
> Can't we separate a bit more the implementations of "clear on read" and
> "clear on write" so that we do not spread the tests that you are talking
> about all over the place and slower the driver's hot paths?

I see. I'll revise my patch. Thank you for your advice.

Regards,
Jongsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-18 Thread Jongsung Kim
On 06/17/2014 04:54 PM, Nicolas Ferre wrote:

Hi Nicolas,

 On 17/06/2014 05:39, Jongsung Kim :
 On 06/17/2014 06:28 AM, Sören Brinkmann wrote:
 Shouldn't it be sufficient to replace 'MACB_BIT(RCOMP) with 
 'MACB_RX_INT_FLAGS'
 to clear all the RX IRQ flags.

 I'm afraid not.

 You know, this driver initially targeted only GEMs configured with 
 gem_irq_clear_read.
 For this implementation of GEM, the ISR is automatically cleared by reading. 
 The driver
 was designed to operate with the value read from ISR, not with the ISR 
 itself.

 However, there are other GEMs configured without gem_irq_clear_read, 
 people like you
 and I working with. To support them, they insert similar codes conditionally 
 clearing
 the ISR here and there. Now they are found at 6 places. Not enough yet. Do 
 you want to
 insert another at the end of macb_reset_hw..? Maybe not.
 
 Can't we separate a bit more the implementations of clear on read and
 clear on write so that we do not spread the tests that you are talking
 about all over the place and slower the driver's hot paths?

I see. I'll revise my patch. Thank you for your advice.

Regards,
Jongsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/17/2014 12:50 PM, Sören Brinkmann wrote:
> On Tue, 2014-06-17 at 11:38AM +0900, Jongsung Kim wrote:
>> On 06/16/2014 11:56 PM, Sören Brinkmann wrote:
>>> On Mon, 2014-06-16 at 02:00PM +0900, Jongsung Kim wrote:
>>>> On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
>>>>> This is now clearing all IRQ flags which is probably not what we want
>>>>> here. This is handling RX only. We still want the non-RX interrupts to go 
>>>>> to
>>>>> the actual interrupt service routing.
>>>>
>>>> The ISR(Interrupt Status Register) is read only in the interrupt service
>>>> routine, macb_interrupt. But is partially cleared here and there. Further
>>>> handler-functions decide jobs to be done by reading/checking other status
>>>> registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
>>>> a bad idea.
>>>
>>> But you are clearing _all_ interrupt flags in the RX NAPI handler.
>>> Doesn't that mean we might miss certain events?
>>
>> Please inspect my patch again. What I did in the macb_poll is removing
>> statements clearing the Rx-complete interrupt, not clearing all the
>> interrupts.
> 
> Why is clearing those bits removed? It's probably not a big hit, but it might
> result in a pointless interrupt which could be avoided. But it should
> probably clear all RX interrupts - MACB_RX_INT_FLAGS - instead of just RCOMP.
> For clear-on-read implementations it shouldn't make a difference.

I agree. But I removed it because I think stepping the same procedure
regardless of the "gem_irq_clear_read" implementation is better than
implementation-specific optimization.

> And in the if-condition in that new helper, I'd add '&& status' to
> avoid writing back zeros.

Good point. I'll add it when I resend v2.

Jongsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/17/2014 06:28 AM, Sören Brinkmann wrote:
> Shouldn't it be sufficient to replace 'MACB_BIT(RCOMP) with 
> 'MACB_RX_INT_FLAGS'
> to clear all the RX IRQ flags.

I'm afraid not.

You know, this driver initially targeted only GEMs configured with 
"gem_irq_clear_read."
For this implementation of GEM, the ISR is automatically cleared by reading. 
The driver
was designed to operate with the value read from ISR, not with the ISR itself.

However, there are other GEMs configured without "gem_irq_clear_read," people 
like you
and I working with. To support them, they insert similar codes conditionally 
clearing
the ISR here and there. Now they are found at 6 places. Not enough yet. Do you 
want to
insert another at the end of macb_reset_hw..? Maybe not.

Jongsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/16/2014 11:56 PM, Sören Brinkmann wrote:
> On Mon, 2014-06-16 at 02:00PM +0900, Jongsung Kim wrote:
>> On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
>>> This is now clearing all IRQ flags which is probably not what we want
>>> here. This is handling RX only. We still want the non-RX interrupts to go to
>>> the actual interrupt service routing.
>>
>> The ISR(Interrupt Status Register) is read only in the interrupt service
>> routine, macb_interrupt. But is partially cleared here and there. Further
>> handler-functions decide jobs to be done by reading/checking other status
>> registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
>> a bad idea.
>
> But you are clearing _all_ interrupt flags in the RX NAPI handler.
> Doesn't that mean we might miss certain events?

Please inspect my patch again. What I did in the macb_poll is removing
statements clearing the Rx-complete interrupt, not clearing all the
interrupts.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/16/2014 11:56 PM, Sören Brinkmann wrote:
 On Mon, 2014-06-16 at 02:00PM +0900, Jongsung Kim wrote:
 On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
 This is now clearing all IRQ flags which is probably not what we want
 here. This is handling RX only. We still want the non-RX interrupts to go to
 the actual interrupt service routing.

 The ISR(Interrupt Status Register) is read only in the interrupt service
 routine, macb_interrupt. But is partially cleared here and there. Further
 handler-functions decide jobs to be done by reading/checking other status
 registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
 a bad idea.

 But you are clearing _all_ interrupt flags in the RX NAPI handler.
 Doesn't that mean we might miss certain events?

Please inspect my patch again. What I did in the macb_poll is removing
statements clearing the Rx-complete interrupt, not clearing all the
interrupts.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/17/2014 06:28 AM, Sören Brinkmann wrote:
 Shouldn't it be sufficient to replace 'MACB_BIT(RCOMP) with 
 'MACB_RX_INT_FLAGS'
 to clear all the RX IRQ flags.

I'm afraid not.

You know, this driver initially targeted only GEMs configured with 
gem_irq_clear_read.
For this implementation of GEM, the ISR is automatically cleared by reading. 
The driver
was designed to operate with the value read from ISR, not with the ISR itself.

However, there are other GEMs configured without gem_irq_clear_read, people 
like you
and I working with. To support them, they insert similar codes conditionally 
clearing
the ISR here and there. Now they are found at 6 places. Not enough yet. Do you 
want to
insert another at the end of macb_reset_hw..? Maybe not.

Jongsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-16 Thread Jongsung Kim
On 06/17/2014 12:50 PM, Sören Brinkmann wrote:
 On Tue, 2014-06-17 at 11:38AM +0900, Jongsung Kim wrote:
 On 06/16/2014 11:56 PM, Sören Brinkmann wrote:
 On Mon, 2014-06-16 at 02:00PM +0900, Jongsung Kim wrote:
 On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
 This is now clearing all IRQ flags which is probably not what we want
 here. This is handling RX only. We still want the non-RX interrupts to go 
 to
 the actual interrupt service routing.

 The ISR(Interrupt Status Register) is read only in the interrupt service
 routine, macb_interrupt. But is partially cleared here and there. Further
 handler-functions decide jobs to be done by reading/checking other status
 registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
 a bad idea.

 But you are clearing _all_ interrupt flags in the RX NAPI handler.
 Doesn't that mean we might miss certain events?

 Please inspect my patch again. What I did in the macb_poll is removing
 statements clearing the Rx-complete interrupt, not clearing all the
 interrupts.
 
 Why is clearing those bits removed? It's probably not a big hit, but it might
 result in a pointless interrupt which could be avoided. But it should
 probably clear all RX interrupts - MACB_RX_INT_FLAGS - instead of just RCOMP.
 For clear-on-read implementations it shouldn't make a difference.

I agree. But I removed it because I think stepping the same procedure
regardless of the gem_irq_clear_read implementation is better than
implementation-specific optimization.

 And in the if-condition in that new helper, I'd add ' status' to
 avoid writing back zeros.

Good point. I'll add it when I resend v2.

Jongsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-15 Thread Jongsung Kim
On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
> Hi Jongsung,

Hi Sören,

> Does this interrupt need to be enabled? There is nothing checking
> that bit and handling this IRQ in the handler, AFAICT. And you solve
> this by simply clearing the bit. So, I wonder whether not enabling this
> IRQ in the first place would solve things too.

The driver actually checks the bit, but does not clear it. Disabling the
"Rx used bit read" interrupt you said may be a solution. However, I think
it is the better way to clear the exceptional HW-state rather than just to
mask it.

> This is now clearing all IRQ flags which is probably not what we want
> here. This is handling RX only. We still want the non-RX interrupts to go to
> the actual interrupt service routing.

The ISR(Interrupt Status Register) is read only in the interrupt service
routine, macb_interrupt. But is partially cleared here and there. Further
handler-functions decide jobs to be done by reading/checking other status
registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
a bad idea.

Jongsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-15 Thread Jongsung Kim
On 06/13/2014 12:44 AM, Sören Brinkmann wrote:
 Hi Jongsung,

Hi Sören,

 Does this interrupt need to be enabled? There is nothing checking
 that bit and handling this IRQ in the handler, AFAICT. And you solve
 this by simply clearing the bit. So, I wonder whether not enabling this
 IRQ in the first place would solve things too.

The driver actually checks the bit, but does not clear it. Disabling the
Rx used bit read interrupt you said may be a solution. However, I think
it is the better way to clear the exceptional HW-state rather than just to
mask it.

 This is now clearing all IRQ flags which is probably not what we want
 here. This is handling RX only. We still want the non-RX interrupts to go to
 the actual interrupt service routing.

The ISR(Interrupt Status Register) is read only in the interrupt service
routine, macb_interrupt. But is partially cleared here and there. Further
handler-functions decide jobs to be done by reading/checking other status
registers. (e.g., TSR, RSR) So, clearing the ISR after reading looks not
a bad idea.

Jongsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-12 Thread Jongsung Kim
The "Rx used bit read" interrupt is enabled but not cleared for some
systems with the ISR (Interrupt Status Register) configured as clear-
on-write. This interrupt may be asserted when the CPU does not handle
Rx-complete interrupts for a long time. (e.g., if the CPU is stopped
by debugger) Once asserted, it'll not be cleared, and the CPU will
loop infinitly in the interrupt handler.

This patch forces to use a dedicated function for reading the ISR,
and the function clears it if clear-on-write. So the ISR is always
cleared after read, regardless of clear-on-write configuration.

Reported-by: Hayun Hwang 
Signed-off-by: Youngkyu Choi 
Signed-off-by: Jongsung Kim 
Tested-by: Hayun Hwang 
---
 drivers/net/ethernet/cadence/macb.c |   37 ++
 1 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index e9daa07..21cc022 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -98,6 +98,16 @@ static void *macb_rx_buffer(struct macb *bp, unsigned int 
index)
return bp->rx_buffers + bp->rx_buffer_size * macb_rx_ring_wrap(index);
 }
 
+static u32 macb_read_isr(struct macb *bp)
+{
+   u32 status = macb_readl(bp, ISR);
+
+   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+   macb_writel(bp, ISR, status);
+
+   return status;
+}
+
 void macb_set_hwaddr(struct macb *bp)
 {
u32 bottom;
@@ -552,9 +562,6 @@ static void macb_tx_interrupt(struct macb *bp)
status = macb_readl(bp, TSR);
macb_writel(bp, TSR, status);
 
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(TCOMP));
-
netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
(unsigned long)status);
 
@@ -883,13 +890,10 @@ static int macb_poll(struct napi_struct *napi, int budget)
 
/* Packets received while interrupts were disabled */
status = macb_readl(bp, RSR);
-   if (status) {
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(RCOMP));
+   if (status)
napi_reschedule(napi);
-   } else {
+   else
macb_writel(bp, IER, MACB_RX_INT_FLAGS);
-   }
}
 
/* TODO: Handle errors */
@@ -903,7 +907,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
struct macb *bp = netdev_priv(dev);
u32 status;
 
-   status = macb_readl(bp, ISR);
+   status = macb_read_isr(bp);
 
if (unlikely(!status))
return IRQ_NONE;
@@ -928,8 +932,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * now.
 */
macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(RCOMP));
 
if (napi_schedule_prep(>napi)) {
netdev_vdbg(bp->dev, "scheduling RX softirq\n");
@@ -941,9 +943,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
schedule_work(>tx_error_task);
 
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_TX_ERR_FLAGS);
-
break;
}
 
@@ -961,9 +960,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
bp->hw_stats.gem.rx_overruns++;
else
bp->hw_stats.macb.rx_overruns++;
-
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(ISR_ROVR));
}
 
if (status & MACB_BIT(HRESP)) {
@@ -973,12 +969,9 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * (work queue?)
 */
netdev_err(dev, "DMA bus error: HRESP not OK\n");
-
-   if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(HRESP));
}
 
-   status = macb_readl(bp, ISR);
+   status = macb_read_isr(bp);
}
 
spin_unlock(>lock);
@@ -1273,7 +1266,7 @@ static void macb_reset_hw(struct macb *bp)
 
/* Disable all interrupts */
macb_writel(bp, IDR, -1);
-   macb_readl(bp, ISR);
+   macb_read_isr(bp);
 }
 
 static u32 gem_mdc_clk_div(struct macb *bp)
-- 
1.7.1

--
To unsubscribe from this 

[PATCH] net/cadence/macb: clear interrupts simply and correctly

2014-06-12 Thread Jongsung Kim
The Rx used bit read interrupt is enabled but not cleared for some
systems with the ISR (Interrupt Status Register) configured as clear-
on-write. This interrupt may be asserted when the CPU does not handle
Rx-complete interrupts for a long time. (e.g., if the CPU is stopped
by debugger) Once asserted, it'll not be cleared, and the CPU will
loop infinitly in the interrupt handler.

This patch forces to use a dedicated function for reading the ISR,
and the function clears it if clear-on-write. So the ISR is always
cleared after read, regardless of clear-on-write configuration.

Reported-by: Hayun Hwang hwang.ha...@lge.com
Signed-off-by: Youngkyu Choi youngkyu7.c...@lge.com
Signed-off-by: Jongsung Kim neidhard@lge.com
Tested-by: Hayun Hwang hwang.ha...@lge.com
---
 drivers/net/ethernet/cadence/macb.c |   37 ++
 1 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index e9daa07..21cc022 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -98,6 +98,16 @@ static void *macb_rx_buffer(struct macb *bp, unsigned int 
index)
return bp-rx_buffers + bp-rx_buffer_size * macb_rx_ring_wrap(index);
 }
 
+static u32 macb_read_isr(struct macb *bp)
+{
+   u32 status = macb_readl(bp, ISR);
+
+   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
+   macb_writel(bp, ISR, status);
+
+   return status;
+}
+
 void macb_set_hwaddr(struct macb *bp)
 {
u32 bottom;
@@ -552,9 +562,6 @@ static void macb_tx_interrupt(struct macb *bp)
status = macb_readl(bp, TSR);
macb_writel(bp, TSR, status);
 
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(TCOMP));
-
netdev_vdbg(bp-dev, macb_tx_interrupt status = 0x%03lx\n,
(unsigned long)status);
 
@@ -883,13 +890,10 @@ static int macb_poll(struct napi_struct *napi, int budget)
 
/* Packets received while interrupts were disabled */
status = macb_readl(bp, RSR);
-   if (status) {
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(RCOMP));
+   if (status)
napi_reschedule(napi);
-   } else {
+   else
macb_writel(bp, IER, MACB_RX_INT_FLAGS);
-   }
}
 
/* TODO: Handle errors */
@@ -903,7 +907,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
struct macb *bp = netdev_priv(dev);
u32 status;
 
-   status = macb_readl(bp, ISR);
+   status = macb_read_isr(bp);
 
if (unlikely(!status))
return IRQ_NONE;
@@ -928,8 +932,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * now.
 */
macb_writel(bp, IDR, MACB_RX_INT_FLAGS);
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(RCOMP));
 
if (napi_schedule_prep(bp-napi)) {
netdev_vdbg(bp-dev, scheduling RX softirq\n);
@@ -941,9 +943,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
macb_writel(bp, IDR, MACB_TX_INT_FLAGS);
schedule_work(bp-tx_error_task);
 
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_TX_ERR_FLAGS);
-
break;
}
 
@@ -961,9 +960,6 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
bp-hw_stats.gem.rx_overruns++;
else
bp-hw_stats.macb.rx_overruns++;
-
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(ISR_ROVR));
}
 
if (status  MACB_BIT(HRESP)) {
@@ -973,12 +969,9 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 * (work queue?)
 */
netdev_err(dev, DMA bus error: HRESP not OK\n);
-
-   if (bp-caps  MACB_CAPS_ISR_CLEAR_ON_WRITE)
-   macb_writel(bp, ISR, MACB_BIT(HRESP));
}
 
-   status = macb_readl(bp, ISR);
+   status = macb_read_isr(bp);
}
 
spin_unlock(bp-lock);
@@ -1273,7 +1266,7 @@ static void macb_reset_hw(struct macb *bp)
 
/* Disable all interrupts */
macb_writel(bp, IDR, -1);
-   macb_readl(bp, ISR);
+   macb_read_isr(bp);
 }
 
 static u32 gem_mdc_clk_div(struct macb *bp)
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message

[PATCH] net: phy: realtek: register/unregister multiple drivers properly

2014-06-09 Thread Jongsung Kim
Using phy_drivers_register/_unregister functions is proper way to
handle multiple PHY drivers registration. For Realtek PHY drivers
module, it fixes incomplete current error-handlings up and adds
missed unregistration for the RTL8201CP driver.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |   88 +++--
 1 files changed, 37 insertions(+), 51 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index fa1d69a..45483fd 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,65 +64,51 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8201CP */
-static struct phy_driver rtl8201cp_driver = {
-   .phy_id = 0x8201,
-   .name   = "RTL8201CP Ethernet",
-   .phy_id_mask= 0x,
-   .features   = PHY_BASIC_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = "RTL8211B Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = "RTL8211E Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drvs[] = {
+   {
+   .phy_id = 0x8201,
+   .name   = "RTL8201CP Ethernet",
+   .phy_id_mask= 0x,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .driver = { .owner = THIS_MODULE,},
+   }, {
+   .phy_id = 0x001cc912,
+   .name   = "RTL8211B Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   }, {
+   .phy_id = 0x001cc915,
+   .name   = "RTL8211E Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(_driver);
-   if (ret < 0)
-   return -ENODEV;
-   ret = phy_driver_register(_driver);
-   if (ret < 0)
-   return -ENODEV;
-   return phy_driver_register(_driver);
+   return phy_drivers_register(realtek_drvs, ARRAY_SIZE(realtek_drvs));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(_driver);
-   phy_driver_unregister(_driver);
+   phy_drivers_unregister(realtek_drvs, ARRAY_SIZE(realtek_drvs));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] net: phy: realtek: register/unregister multiple drivers properly

2014-06-09 Thread Jongsung Kim
Using phy_drivers_register/_unregister functions is proper way to
handle multiple PHY drivers registration. For Realtek PHY drivers
module, it fixes incomplete current error-handlings up and adds
missed unregistration for the RTL8201CP driver.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |   88 +++--
 1 files changed, 37 insertions(+), 51 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index fa1d69a..45483fd 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,65 +64,51 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8201CP */
-static struct phy_driver rtl8201cp_driver = {
-   .phy_id = 0x8201,
-   .name   = RTL8201CP Ethernet,
-   .phy_id_mask= 0x,
-   .features   = PHY_BASIC_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = RTL8211B Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211b_config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = RTL8211E Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211e_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drvs[] = {
+   {
+   .phy_id = 0x8201,
+   .name   = RTL8201CP Ethernet,
+   .phy_id_mask= 0x,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .driver = { .owner = THIS_MODULE,},
+   }, {
+   .phy_id = 0x001cc912,
+   .name   = RTL8211B Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211b_config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   }, {
+   .phy_id = 0x001cc915,
+   .name   = RTL8211E Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211e_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(rtl8201cp_driver);
-   if (ret  0)
-   return -ENODEV;
-   ret = phy_driver_register(rtl8211b_driver);
-   if (ret  0)
-   return -ENODEV;
-   return phy_driver_register(rtl8211e_driver);
+   return phy_drivers_register(realtek_drvs, ARRAY_SIZE(realtek_drvs));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(rtl8211b_driver);
-   phy_driver_unregister(rtl8211e_driver);
+   phy_drivers_unregister(realtek_drvs, ARRAY_SIZE(realtek_drvs));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] net/cadence/macb: fix bug/typo in extracting gem_irq_read_clear bit

2013-07-09 Thread Jongsung Kim
Signed-off-by: Jongsung Kim 
---
 drivers/net/ethernet/cadence/macb.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index c89aa41..b4e0dc8 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1070,7 +1070,7 @@ static void macb_configure_dma(struct macb *bp)
 static void macb_configure_caps(struct macb *bp)
 {
if (macb_is_gem(bp)) {
-   if (GEM_BF(IRQCOR, gem_readl(bp, DCFG1)) == 0)
+   if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
}
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] net/cadence/macb: fix bug/typo in extracting gem_irq_read_clear bit

2013-07-09 Thread Jongsung Kim
Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/ethernet/cadence/macb.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c 
b/drivers/net/ethernet/cadence/macb.c
index c89aa41..b4e0dc8 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1070,7 +1070,7 @@ static void macb_configure_dma(struct macb *bp)
 static void macb_configure_caps(struct macb *bp)
 {
if (macb_is_gem(bp)) {
-   if (GEM_BF(IRQCOR, gem_readl(bp, DCFG1)) == 0)
+   if (GEM_BFEXT(IRQCOR, gem_readl(bp, DCFG1)) == 0)
bp-caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
}
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Stephen Warren  :
> Looking at the TRM, it seems this is really the only change, according
> to the changelog in the documentation (although it's a little difficult
> to tell since the document seems to have a bunch of changes that
presumably
> don't affect behaviour). So, faking the periphid seems OK.
>
> Acked-by: Stephen Warren 
>
> Let's apply for 3.10.

Thank you, Stephen.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Jongsung Kim  :
> diff --git a/arch/arm/boot/dts/bcm2835.dtsi
b/arch/arm/boot/dts/bcm2835.dtsi
> index f0052dc..1e12aef 100644
> --- a/arch/arm/boot/dts/bcm2835.dtsi
> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> @@ -44,6 +44,7 @@
>   reg = <0x7e201000 0x1000>;
>   interrupts = <2 25>;
>   clock-frequency = <300>;
> + arm,primecell-periphid = <0x00241011>;
>   };
>  
>   gpio: gpio {

Stephen, how do you think about this kind of approach instead?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Stephen Warren reported the recent commit 78506f2 (add support for
extended FIFO-size of PL011-r1p5) breaks the serial port on the
BCM2835 ARM SoC.

A UART compatible with the ARM PL011-r1p5 should have 32-deep FIFOs.
The BCM2835 UART just looks like an ARM PL011-r1p5, but has 16-deep
FIFOs just like PL011-r1p4 or earlier revisions. As a workaround for
this compatibility issue, this patch overrides the HW UART periphid
register values with the actually compatible UART periphid 0x00241011
(r1p3 or r1p4).

Reported-by: Stephen Warren 
Signed-off-by: Jongsung Kim 
---
 arch/arm/boot/dts/bcm2835.dtsi |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index f0052dc..1e12aef 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -44,6 +44,7 @@
reg = <0x7e201000 0x1000>;
interrupts = <2 25>;
clock-frequency = <300>;
+   arm,primecell-periphid = <0x00241011>;
};
 
gpio: gpio {
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Stephen Warren reported the recent commit 78506f2 (add support for
extended FIFO-size of PL011-r1p5) breaks the serial port on the
BCM2835 ARM SoC.

A UART compatible with the ARM PL011-r1p5 should have 32-deep FIFOs.
The BCM2835 UART just looks like an ARM PL011-r1p5, but has 16-deep
FIFOs just like PL011-r1p4 or earlier revisions. As a workaround for
this compatibility issue, this patch overrides the HW UART periphid
register values with the actually compatible UART periphid 0x00241011
(r1p3 or r1p4).

Reported-by: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Jongsung Kim neidhard@lge.com
---
 arch/arm/boot/dts/bcm2835.dtsi |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index f0052dc..1e12aef 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -44,6 +44,7 @@
reg = 0x7e201000 0x1000;
interrupts = 2 25;
clock-frequency = 300;
+   arm,primecell-periphid = 0x00241011;
};
 
gpio: gpio {
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Jongsung Kim neidhard@lge.com :
 diff --git a/arch/arm/boot/dts/bcm2835.dtsi
b/arch/arm/boot/dts/bcm2835.dtsi
 index f0052dc..1e12aef 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -44,6 +44,7 @@
   reg = 0x7e201000 0x1000;
   interrupts = 2 25;
   clock-frequency = 300;
 + arm,primecell-periphid = 0x00241011;
   };
  
   gpio: gpio {

Stephen, how do you think about this kind of approach instead?

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: bcm2835: override the HW UART periphid

2013-05-21 Thread Jongsung Kim
Stephen Warren swar...@wwwdotorg.org :
 Looking at the TRM, it seems this is really the only change, according
 to the changelog in the documentation (although it's a little difficult
 to tell since the document seems to have a bunch of changes that
presumably
 don't affect behaviour). So, faking the periphid seems OK.

 Acked-by: Stephen Warren swar...@wwwdotorg.org

 Let's apply for 3.10.

Thank you, Stephen.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-20 Thread Jongsung Kim
Jongsung Kim  :
> Stephen Warren  :
>>> All r1p5 have 32-byte FIFO depth and it's not configurable. From the 
>>> PL011
>>> TRM:
>>> 
>>> r1p4-r1p5   Contains the following differences in functionality:
>>> * The receive and transmit FIFOs are increased to a depth of
32.
>>> * The Revision field in the UARTPeriphID2 Register on page
3-24
>>>   bits [7:4] now reads back as 0x3.
>>
>> Well, that certainly isn't true in practice. I think we should revert 
>> this commit until we can determine what the problem is.
>
> I asked to the ARM support about this. Waiting for reply..

ARM support said they doesn't have information about BCM2835 UART. Does
anyone have a communication channel to Broadcom? It takes time for me to
get contact point to Broadcom.. (I'm trying)

However, ARM support also said:

"If the Broadcom part definitely has 16-deep FIFOs, it cannot be based
on a PL011 r1p5, so I might guess that Broadcom have just referenced
the latest version of the documentation on our website, but have actually
implemented an earlier version."

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-20 Thread Jongsung Kim
Jongsung Kim neidhard@lge.com :
 Stephen Warren swar...@wwwdotorg.org :
 All r1p5 have 32-byte FIFO depth and it's not configurable. From the 
 PL011
 TRM:
 
 r1p4-r1p5   Contains the following differences in functionality:
 * The receive and transmit FIFOs are increased to a depth of
32.
 * The Revision field in the UARTPeriphID2 Register on page
3-24
   bits [7:4] now reads back as 0x3.

 Well, that certainly isn't true in practice. I think we should revert 
 this commit until we can determine what the problem is.

 I asked to the ARM support about this. Waiting for reply..

ARM support said they doesn't have information about BCM2835 UART. Does
anyone have a communication channel to Broadcom? It takes time for me to
get contact point to Broadcom.. (I'm trying)

However, ARM support also said:

If the Broadcom part definitely has 16-deep FIFOs, it cannot be based
on a PL011 r1p5, so I might guess that Broadcom have just referenced
the latest version of the documentation on our website, but have actually
implemented an earlier version.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-16 Thread Jongsung Kim
Stephen Warren  :
>> All r1p5 have 32-byte FIFO depth and it's not configurable. From the
PL011
>> TRM:
>> 
>> r1p4-r1p5Contains the following differences in functionality:
>>  * The receive and transmit FIFOs are increased to a depth of
32.
>>  * The Revision field in the UARTPeriphID2 Register on page
3-24
>>bits [7:4] now reads back as 0x3.
>
> Well, that certainly isn't true in practice. I think we should revert
> this commit until we can determine what the problem is.

I asked to the ARM support about this. Waiting for reply..

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-16 Thread Jongsung Kim
Stephen Warren swar...@wwwdotorg.org :
 All r1p5 have 32-byte FIFO depth and it's not configurable. From the
PL011
 TRM:
 
 r1p4-r1p5Contains the following differences in functionality:
  * The receive and transmit FIFOs are increased to a depth of
32.
  * The Revision field in the UARTPeriphID2 Register on page
3-24
bits [7:4] now reads back as 0x3.

 Well, that certainly isn't true in practice. I think we should revert
 this commit until we can determine what the problem is.

I asked to the ARM support about this. Waiting for reply..

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-14 Thread Jongsung Kim
Stephen Warren  :
> Looking at BCM2835-ARM-Peripherals.pdf (i.e. the public documentation for
> the BCM2835 chip), I see:
>
> =
> The UART provides:
> * Separate 16x8 transmit and 16x12 receive FIFO memory.
> ...
> For the in-depth UART overview, please, refer to the ARM PrimeCell UART
> (PL011) Revision: r1p5 Technical Reference Manual.
> =
>
> That seems to imply that not all r1p5 PL011s actually have a depth-32
FIFO.
> Perhaps this is a configurable property of the IP block, not something
that
> all r1p5 have?

All r1p5 have 32-byte FIFO depth and it's not configurable. From the PL011
TRM:

r1p4-r1p5   Contains the following differences in functionality:
* The receive and transmit FIFOs are increased to a depth of
32.
* The Revision field in the UARTPeriphID2 Register on page
3-24
  bits [7:4] now reads back as 0x3.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-14 Thread Jongsung Kim
Stephen Warren  :
> For reference, the AMBA periphid of the UART device there is 0x00341011.
> The nibble "3" is the revision being tested in:

The UART device has periphid 0x00341011, and is compatible with the
original PL011 prior to r1p5. Not with r1p5. It could be a possible
way to specify the compatible periphid (such as 0x00241011) instead
of just 0x0 when initializing the amba_device for the UART.

> > +static unsigned int get_fifosize_arm(unsigned int periphid)
> > +{
> > +   unsigned int rev = (periphid >> 20) & 0xf;
> > +   return rev < 3 ? 16 : 32;
> > +}
>
> Should that be <= not <, or is there just something more wrong in the
> patch or bcm2835 HW? I wonder how r1p5 maps to 3 in the test above.

>From the PL011-r1p5 TRM, bits[7:4] of the UARTPeriphID2 register are
read as:

r1p0 - 0x0
r1p1 - 0x1
r1p3 - 0x2
r1p4 - 0x2
r1p5 - 0x3.

Doesn't the BCM2835 UART have anything different from the ARM PL011?
What about the UARTPCellID registers? They are set to 0xb105f00d with
the ARM PL011.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-14 Thread Jongsung Kim
Stephen Warren swar...@wwwdotorg.org :
 For reference, the AMBA periphid of the UART device there is 0x00341011.
 The nibble 3 is the revision being tested in:

The UART device has periphid 0x00341011, and is compatible with the
original PL011 prior to r1p5. Not with r1p5. It could be a possible
way to specify the compatible periphid (such as 0x00241011) instead
of just 0x0 when initializing the amba_device for the UART.

  +static unsigned int get_fifosize_arm(unsigned int periphid)
  +{
  +   unsigned int rev = (periphid  20)  0xf;
  +   return rev  3 ? 16 : 32;
  +}

 Should that be = not , or is there just something more wrong in the
 patch or bcm2835 HW? I wonder how r1p5 maps to 3 in the test above.

From the PL011-r1p5 TRM, bits[7:4] of the UARTPeriphID2 register are
read as:

r1p0 - 0x0
r1p1 - 0x1
r1p3 - 0x2
r1p4 - 0x2
r1p5 - 0x3.

Doesn't the BCM2835 UART have anything different from the ARM PL011?
What about the UARTPCellID registers? They are set to 0xb105f00d with
the ARM PL011.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

2013-05-14 Thread Jongsung Kim
Stephen Warren swar...@wwwdotorg.org :
 Looking at BCM2835-ARM-Peripherals.pdf (i.e. the public documentation for
 the BCM2835 chip), I see:

 =
 The UART provides:
 * Separate 16x8 transmit and 16x12 receive FIFO memory.
 ...
 For the in-depth UART overview, please, refer to the ARM PrimeCell UART
 (PL011) Revision: r1p5 Technical Reference Manual.
 =

 That seems to imply that not all r1p5 PL011s actually have a depth-32
FIFO.
 Perhaps this is a configurable property of the IP block, not something
that
 all r1p5 have?

All r1p5 have 32-byte FIFO depth and it's not configurable. From the PL011
TRM:

r1p4-r1p5   Contains the following differences in functionality:
* The receive and transmit FIFOs are increased to a depth of
32.
* The Revision field in the UARTPeriphID2 Register on page
3-24
  bits [7:4] now reads back as 0x3.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 4/4] net: phy: realtek: cleanup code

2013-05-13 Thread Jongsung Kim
This patch cleans up the drivers code by:

- using a consistent way to reference functions
- removing unused macro-definitions
- removing unnecessary new-lines
- making ack_interrupt functions shorter.

Changes in v3:
* don't use the '&' to reference functions (pointed by Sergei Shtylyov)

Changes in v2:
* separated patch (pointed by Francois Romieu and Sergei Shtylyov)
* make functions shorter (pointed by Sergei Shtylyov)

Signed-off-by: Jongsung Kim 
Cc: Francois Romieu 
Cc: Sergei Shtylyov 
---
 drivers/net/phy/realtek.c |   41 -
 1 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 6f0726a..b55aea2 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -21,9 +21,6 @@
 #define RTL8201F_INER  0x13
 #define RTL8201F_INER_MASK 0x3800
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -36,18 +33,14 @@ MODULE_LICENSE("GPL");
 
 static int rtl8201f_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL8201F_INSR);
+   int err = phy_read(phydev, RTL8201F_INSR);
 
return (err < 0) ? err : 0;
 }
 
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL821x_INSR);
+   int err = phy_read(phydev, RTL821x_INSR);
 
return (err < 0) ? err : 0;
 }
@@ -80,8 +73,7 @@ static int rtl8211b_config_intr(struct phy_device *phydev)
int err;
 
if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL821x_INER_INIT);
+   err = phy_write(phydev, RTL821x_INER, RTL821x_INER_INIT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -93,8 +85,7 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
int err;
 
if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL8211E_INER_LINK_STAT);
+   err = phy_write(phydev, RTL821x_INER, RTL8211E_INER_LINK_STAT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -108,10 +99,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_BASIC_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl8201f_ack_interrupt,
+   .config_intr= rtl8201f_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -122,10 +113,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_GBIT_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211b_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -136,10 +127,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_GBIT_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211e_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More major

[PATCH v3 2/4] net: phy: realtek: add support for the RTL8201F

2013-05-13 Thread Jongsung Kim
Add the minimal driver to support the Realtek RTL8201F 10/100Mbps
Ethernet Transceivers.

The help message for the config REALTEK_PHY is also modified to
describe the current status of the module correctly.

Changes in v2:
* separated patch (pointed by Francois Romieu and Sergei Shtylyov)
* create register page-selecting function (pointed by Francois Romieu)

Signed-off-by: Jongsung Kim 
Cc: Francois Romieu 
Cc: Sergei Shtylyov 
---
 drivers/net/phy/Kconfig   |2 +-
 drivers/net/phy/realtek.c |   52 +
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1e11f2b..000425e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,7 +84,7 @@ config ICPLUS_PHY
 config REALTEK_PHY
tristate "Drivers for Realtek PHYs"
---help---
- Supports the Realtek 821x PHY.
+ Currently supports the RTL8201F, RTL8211B and RTL8211E PHYs.
 
 config NATIONAL_PHY
tristate "Drivers for National Semiconductor PHYs"
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f902107..8e95e38 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,6 +16,11 @@
 #include 
 #include 
 
+#define RTL8201F_INSR  0x1e
+#define RTL8201F_PGSR  0x1f
+#define RTL8201F_INER  0x13
+#define RTL8201F_INER_MASK 0x3800
+
 #define RTL821x_PHYSR  0x11
 #define RTL821x_PHYSR_DUPLEX   0x2000
 #define RTL821x_PHYSR_SPEED0xc000
@@ -29,6 +34,15 @@ MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_INSR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +52,29 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static void rtl8201f_select_page(struct phy_device *phydev, int page)
+{
+   phy_write(phydev, RTL8201F_PGSR, page);
+}
+
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   rtl8201f_select_page(phydev, 7);
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_INER, RTL8201F_INER_MASK |
+   phy_read(phydev, RTL8201F_INER));
+   else
+   err = phy_write(phydev, RTL8201F_INER, ~RTL8201F_INER_MASK &
+   phy_read(phydev, RTL8201F_INER));
+
+   rtl8201f_select_page(phydev, 0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -65,6 +102,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
 }
 
 static struct phy_driver realtek_drv[] = {
+   {   /* RTL8201F */
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
{   /* RTL8211B */
.phy_id = 0x001cc912,
.name   = "RTL8211B Gigabit Ethernet",
@@ -107,6 +158,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 3/4] net: phy: realtek: add suspend/resume handlers for the RTL8211B

2013-05-13 Thread Jongsung Kim
Add compatible but missing generic PHY suspend/resume handlers for
the RTL8211B driver.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e95e38..6f0726a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -126,6 +126,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= _read_status,
.ack_interrupt  = _ack_interrupt,
.config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/4] net: phy: realtek: simplify multiple drivers registration

2013-05-13 Thread Jongsung Kim
Use the phy_drivers_register and phy_drivers_unregister to
simplify registration and error handling. The two existing
phy_driver structures are converted to an array of the
structures to do this.

v2: use proper way to register multiple drivers.

Signed-off-by: Jongsung Kim 
Cc: Francois Romieu 
---
 drivers/net/phy/realtek.c |   65 -
 1 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..f902107 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,50 +64,43 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = "RTL8211B Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = "RTL8211E Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drv[] = {
+   {   /* RTL8211B */
+   .phy_id = 0x001cc912,
+   .name   = "RTL8211B Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   },
+   {   /* RTL8211E */
+   .phy_id = 0x001cc915,
+   .name   = "RTL8211E Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(_driver);
-   if (ret < 0)
-   return -ENODEV;
-   return phy_driver_register(_driver);
+   return phy_drivers_register(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(_driver);
-   phy_driver_unregister(_driver);
+   phy_drivers_unregister(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/4] net: phy: realtek: simplify multiple drivers registration

2013-05-13 Thread Jongsung Kim
Use the phy_drivers_register and phy_drivers_unregister to
simplify registration and error handling. The two existing
phy_driver structures are converted to an array of the
structures to do this.

v2: use proper way to register multiple drivers.

Signed-off-by: Jongsung Kim neidhard@lge.com
Cc: Francois Romieu rom...@fr.zoreil.com
---
 drivers/net/phy/realtek.c |   65 -
 1 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..f902107 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,50 +64,43 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = RTL8211B Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211b_config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = RTL8211E Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211e_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drv[] = {
+   {   /* RTL8211B */
+   .phy_id = 0x001cc912,
+   .name   = RTL8211B Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211b_config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   },
+   {   /* RTL8211E */
+   .phy_id = 0x001cc915,
+   .name   = RTL8211E Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211e_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(rtl8211b_driver);
-   if (ret  0)
-   return -ENODEV;
-   return phy_driver_register(rtl8211e_driver);
+   return phy_drivers_register(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(rtl8211b_driver);
-   phy_driver_unregister(rtl8211e_driver);
+   phy_drivers_unregister(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 2/4] net: phy: realtek: add support for the RTL8201F

2013-05-13 Thread Jongsung Kim
Add the minimal driver to support the Realtek RTL8201F 10/100Mbps
Ethernet Transceivers.

The help message for the config REALTEK_PHY is also modified to
describe the current status of the module correctly.

Changes in v2:
* separated patch (pointed by Francois Romieu and Sergei Shtylyov)
* create register page-selecting function (pointed by Francois Romieu)

Signed-off-by: Jongsung Kim neidhard@lge.com
Cc: Francois Romieu rom...@fr.zoreil.com
Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
 drivers/net/phy/Kconfig   |2 +-
 drivers/net/phy/realtek.c |   52 +
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1e11f2b..000425e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,7 +84,7 @@ config ICPLUS_PHY
 config REALTEK_PHY
tristate Drivers for Realtek PHYs
---help---
- Supports the Realtek 821x PHY.
+ Currently supports the RTL8201F, RTL8211B and RTL8211E PHYs.
 
 config NATIONAL_PHY
tristate Drivers for National Semiconductor PHYs
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f902107..8e95e38 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,6 +16,11 @@
 #include linux/phy.h
 #include linux/module.h
 
+#define RTL8201F_INSR  0x1e
+#define RTL8201F_PGSR  0x1f
+#define RTL8201F_INER  0x13
+#define RTL8201F_INER_MASK 0x3800
+
 #define RTL821x_PHYSR  0x11
 #define RTL821x_PHYSR_DUPLEX   0x2000
 #define RTL821x_PHYSR_SPEED0xc000
@@ -29,6 +34,15 @@ MODULE_DESCRIPTION(Realtek PHY driver);
 MODULE_AUTHOR(Johnson Leung);
 MODULE_LICENSE(GPL);
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_INSR);
+
+   return (err  0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +52,29 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err  0) ? err : 0;
 }
 
+static void rtl8201f_select_page(struct phy_device *phydev, int page)
+{
+   phy_write(phydev, RTL8201F_PGSR, page);
+}
+
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   rtl8201f_select_page(phydev, 7);
+
+   if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_INER, RTL8201F_INER_MASK |
+   phy_read(phydev, RTL8201F_INER));
+   else
+   err = phy_write(phydev, RTL8201F_INER, ~RTL8201F_INER_MASK 
+   phy_read(phydev, RTL8201F_INER));
+
+   rtl8201f_select_page(phydev, 0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -65,6 +102,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
 }
 
 static struct phy_driver realtek_drv[] = {
+   {   /* RTL8201F */
+   .phy_id = 0x001cc816,
+   .name   = RTL8201F 10/100Mbps Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl8201f_ack_interrupt,
+   .config_intr= rtl8201f_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
{   /* RTL8211B */
.phy_id = 0x001cc912,
.name   = RTL8211B Gigabit Ethernet,
@@ -107,6 +158,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 3/4] net: phy: realtek: add suspend/resume handlers for the RTL8211B

2013-05-13 Thread Jongsung Kim
Add compatible but missing generic PHY suspend/resume handlers for
the RTL8211B driver.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e95e38..6f0726a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -126,6 +126,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= genphy_read_status,
.ack_interrupt  = rtl821x_ack_interrupt,
.config_intr= rtl8211b_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 4/4] net: phy: realtek: cleanup code

2013-05-13 Thread Jongsung Kim
This patch cleans up the drivers code by:

- using a consistent way to reference functions
- removing unused macro-definitions
- removing unnecessary new-lines
- making ack_interrupt functions shorter.

Changes in v3:
* don't use the '' to reference functions (pointed by Sergei Shtylyov)

Changes in v2:
* separated patch (pointed by Francois Romieu and Sergei Shtylyov)
* make functions shorter (pointed by Sergei Shtylyov)

Signed-off-by: Jongsung Kim neidhard@lge.com
Cc: Francois Romieu rom...@fr.zoreil.com
Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
 drivers/net/phy/realtek.c |   41 -
 1 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 6f0726a..b55aea2 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -21,9 +21,6 @@
 #define RTL8201F_INER  0x13
 #define RTL8201F_INER_MASK 0x3800
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -36,18 +33,14 @@ MODULE_LICENSE(GPL);
 
 static int rtl8201f_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL8201F_INSR);
+   int err = phy_read(phydev, RTL8201F_INSR);
 
return (err  0) ? err : 0;
 }
 
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL821x_INSR);
+   int err = phy_read(phydev, RTL821x_INSR);
 
return (err  0) ? err : 0;
 }
@@ -80,8 +73,7 @@ static int rtl8211b_config_intr(struct phy_device *phydev)
int err;
 
if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL821x_INER_INIT);
+   err = phy_write(phydev, RTL821x_INER, RTL821x_INER_INIT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -93,8 +85,7 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
int err;
 
if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL8211E_INER_LINK_STAT);
+   err = phy_write(phydev, RTL821x_INER, RTL8211E_INER_LINK_STAT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -108,10 +99,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_BASIC_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl8201f_ack_interrupt,
-   .config_intr= rtl8201f_config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl8201f_ack_interrupt,
+   .config_intr= rtl8201f_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -122,10 +113,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_GBIT_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211b_config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211b_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -136,10 +127,10 @@ static struct phy_driver realtek_drv[] = {
.phy_id_mask= 0x001f,
.features   = PHY_GBIT_FEATURES,
.flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211e_config_intr,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211e_config_intr,
.suspend= genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
-- 
1.7.1

--
To unsubscribe from this list

RE: [PATCH v2 4/4] net: phy: realtek: cleanup code

2013-05-12 Thread Jongsung Kim
Sergei Shtylyov  :
>> -.suspend= genphy_suspend,
>> -.resume = genphy_resume,
>> +.suspend= _suspend,
>> +.resume = _resume,
>
>Contrariwise, you should have dropped & from the other functions. 
>It's completely superfluous.
>

Agreed. Thank you  for your comment, Sergei.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH v2 4/4] net: phy: realtek: cleanup code

2013-05-12 Thread Jongsung Kim
Sergei Shtylyov sergei.shtyl...@cogentembedded.com :
 -.suspend= genphy_suspend,
 -.resume = genphy_resume,
 +.suspend= genphy_suspend,
 +.resume = genphy_resume,

Contrariwise, you should have dropped  from the other functions. 
It's completely superfluous.


Agreed. Thank you  for your comment, Sergei.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RESEND] tty: serial: amba-pl011: revise to use amba_rev macro

2013-05-10 Thread Jongsung Kim
This patch replaces the ugly bit-operations with the convenient
amba_rev macro from the get_fifosize_arm function. The type of
get_fifosize member function is also slightly changed to take
a pointer to the amba_device.

Signed-off-by: Jongsung Kim 
---
 drivers/tty/serial/amba-pl011.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8ab70a6..85cfe11 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -79,13 +79,12 @@ struct vendor_data {
booldma_threshold;
boolcts_event_workaround;
 
-   unsigned int (*get_fifosize)(unsigned int periphid);
+   unsigned int (*get_fifosize)(struct amba_device *dev);
 };
 
-static unsigned int get_fifosize_arm(unsigned int periphid)
+static unsigned int get_fifosize_arm(struct amba_device *dev)
 {
-   unsigned int rev = (periphid >> 20) & 0xf;
-   return rev < 3 ? 16 : 32;
+   return amba_rev(dev) < 3 ? 16 : 32;
 }
 
 static struct vendor_data vendor_arm = {
@@ -98,7 +97,7 @@ static struct vendor_data vendor_arm = {
.get_fifosize   = get_fifosize_arm,
 };
 
-static unsigned int get_fifosize_st(unsigned int periphid)
+static unsigned int get_fifosize_st(struct amba_device *dev)
 {
return 64;
 }
@@ -2157,7 +2156,7 @@ static int pl011_probe(struct amba_device *dev, const 
struct amba_id *id)
uap->lcrh_rx = vendor->lcrh_rx;
uap->lcrh_tx = vendor->lcrh_tx;
uap->old_cr = 0;
-   uap->fifosize = vendor->get_fifosize(dev->periphid);
+   uap->fifosize = vendor->get_fifosize(dev);
uap->port.dev = >dev;
uap->port.mapbase = dev->res.start;
uap->port.membase = base;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 1/4] net: phy: realtek: simplify multiple drivers registration

2013-05-10 Thread Jongsung Kim
Use the phy_drivers_register and phy_drivers_unregister to
simplify registration and error handling. The two existing
phy_driver structures are converted to an array of the
structures to do this.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |   65 -
 1 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..f902107 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,50 +64,43 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = "RTL8211B Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = "RTL8211E Gigabit Ethernet",
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= _config_aneg,
-   .read_status= _read_status,
-   .ack_interrupt  = _ack_interrupt,
-   .config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drv[] = {
+   {   /* RTL8211B */
+   .phy_id = 0x001cc912,
+   .name   = "RTL8211B Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   },
+   {   /* RTL8211E */
+   .phy_id = 0x001cc915,
+   .name   = "RTL8211E Gigabit Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(_driver);
-   if (ret < 0)
-   return -ENODEV;
-   return phy_driver_register(_driver);
+   return phy_drivers_register(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(_driver);
-   phy_driver_unregister(_driver);
+   phy_drivers_unregister(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/4] net: phy: realtek: add suspend/resume handlers for the RTL8211B

2013-05-10 Thread Jongsung Kim
Add compatible but missing generic PHY suspend/resume handlers for
the RTL8211B driver.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e95e38..6f0726a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -126,6 +126,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= _read_status,
.ack_interrupt  = _ack_interrupt,
.config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 2/4] net: phy: realtek: add support for the RTL8201F

2013-05-10 Thread Jongsung Kim
Add the minimal driver to support the Realtek RTL8201F 10/100Mbps
Ethernet Transceivers.

The help message for the config REALTEK_PHY is also modified to
describe the current status of the module correctly.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/Kconfig   |2 +-
 drivers/net/phy/realtek.c |   52 +
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1e11f2b..000425e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,7 +84,7 @@ config ICPLUS_PHY
 config REALTEK_PHY
tristate "Drivers for Realtek PHYs"
---help---
- Supports the Realtek 821x PHY.
+ Currently supports the RTL8201F, RTL8211B and RTL8211E PHYs.
 
 config NATIONAL_PHY
tristate "Drivers for National Semiconductor PHYs"
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f902107..8e95e38 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,6 +16,11 @@
 #include 
 #include 
 
+#define RTL8201F_INSR  0x1e
+#define RTL8201F_PGSR  0x1f
+#define RTL8201F_INER  0x13
+#define RTL8201F_INER_MASK 0x3800
+
 #define RTL821x_PHYSR  0x11
 #define RTL821x_PHYSR_DUPLEX   0x2000
 #define RTL821x_PHYSR_SPEED0xc000
@@ -29,6 +34,15 @@ MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_INSR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +52,29 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static void rtl8201f_select_page(struct phy_device *phydev, int page)
+{
+   phy_write(phydev, RTL8201F_PGSR, page);
+}
+
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   rtl8201f_select_page(phydev, 7);
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_INER, RTL8201F_INER_MASK |
+   phy_read(phydev, RTL8201F_INER));
+   else
+   err = phy_write(phydev, RTL8201F_INER, ~RTL8201F_INER_MASK &
+   phy_read(phydev, RTL8201F_INER));
+
+   rtl8201f_select_page(phydev, 0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -65,6 +102,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
 }
 
 static struct phy_driver realtek_drv[] = {
+   {   /* RTL8201F */
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
{   /* RTL8211B */
.phy_id = 0x001cc912,
.name   = "RTL8211B Gigabit Ethernet",
@@ -107,6 +158,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 4/4] net: phy: realtek: cleanup code

2013-05-10 Thread Jongsung Kim
This patch cleans up the drivers code by:

- using a consistent way to reference functions
- removing unused macro-definitions
- removing unnecessary new-lines
- making ack_interrupt functions shorter.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |   29 ++---
 1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 6f0726a..fd09844 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -21,9 +21,6 @@
 #define RTL8201F_INER  0x13
 #define RTL8201F_INER_MASK 0x3800
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -36,18 +33,14 @@ MODULE_LICENSE("GPL");
 
 static int rtl8201f_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL8201F_INSR);
+   int err = phy_read(phydev, RTL8201F_INSR);
 
return (err < 0) ? err : 0;
 }
 
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL821x_INSR);
+   int err = phy_read(phydev, RTL821x_INSR);
 
return (err < 0) ? err : 0;
 }
@@ -80,8 +73,7 @@ static int rtl8211b_config_intr(struct phy_device *phydev)
int err;
 
if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL821x_INER_INIT);
+   err = phy_write(phydev, RTL821x_INER, RTL821x_INER_INIT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -93,8 +85,7 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
int err;
 
if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL8211E_INER_LINK_STAT);
+   err = phy_write(phydev, RTL821x_INER, RTL8211E_INER_LINK_STAT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -112,8 +103,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= _read_status,
.ack_interrupt  = _ack_interrupt,
.config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= _suspend,
+   .resume = _resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211B */
@@ -126,8 +117,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= _read_status,
.ack_interrupt  = _ack_interrupt,
.config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= _suspend,
+   .resume = _resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
@@ -140,8 +131,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= _read_status,
.ack_interrupt  = _ack_interrupt,
.config_intr= _config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= _suspend,
+   .resume = _resume,
.driver = { .owner = THIS_MODULE,},
},
 };
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 4/4] net: phy: realtek: cleanup code

2013-05-10 Thread Jongsung Kim
This patch cleans up the drivers code by:

- using a consistent way to reference functions
- removing unused macro-definitions
- removing unnecessary new-lines
- making ack_interrupt functions shorter.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |   29 ++---
 1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 6f0726a..fd09844 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -21,9 +21,6 @@
 #define RTL8201F_INER  0x13
 #define RTL8201F_INER_MASK 0x3800
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -36,18 +33,14 @@ MODULE_LICENSE(GPL);
 
 static int rtl8201f_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL8201F_INSR);
+   int err = phy_read(phydev, RTL8201F_INSR);
 
return (err  0) ? err : 0;
 }
 
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
-   int err;
-
-   err = phy_read(phydev, RTL821x_INSR);
+   int err = phy_read(phydev, RTL821x_INSR);
 
return (err  0) ? err : 0;
 }
@@ -80,8 +73,7 @@ static int rtl8211b_config_intr(struct phy_device *phydev)
int err;
 
if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL821x_INER_INIT);
+   err = phy_write(phydev, RTL821x_INER, RTL821x_INER_INIT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -93,8 +85,7 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
int err;
 
if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
-   err = phy_write(phydev, RTL821x_INER,
-   RTL8211E_INER_LINK_STAT);
+   err = phy_write(phydev, RTL821x_INER, RTL8211E_INER_LINK_STAT);
else
err = phy_write(phydev, RTL821x_INER, 0);
 
@@ -112,8 +103,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= genphy_read_status,
.ack_interrupt  = rtl8201f_ack_interrupt,
.config_intr= rtl8201f_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211B */
@@ -126,8 +117,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= genphy_read_status,
.ack_interrupt  = rtl821x_ack_interrupt,
.config_intr= rtl8211b_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
@@ -140,8 +131,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= genphy_read_status,
.ack_interrupt  = rtl821x_ack_interrupt,
.config_intr= rtl8211e_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
 };
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 2/4] net: phy: realtek: add support for the RTL8201F

2013-05-10 Thread Jongsung Kim
Add the minimal driver to support the Realtek RTL8201F 10/100Mbps
Ethernet Transceivers.

The help message for the config REALTEK_PHY is also modified to
describe the current status of the module correctly.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/Kconfig   |2 +-
 drivers/net/phy/realtek.c |   52 +
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1e11f2b..000425e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -84,7 +84,7 @@ config ICPLUS_PHY
 config REALTEK_PHY
tristate Drivers for Realtek PHYs
---help---
- Supports the Realtek 821x PHY.
+ Currently supports the RTL8201F, RTL8211B and RTL8211E PHYs.
 
 config NATIONAL_PHY
tristate Drivers for National Semiconductor PHYs
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index f902107..8e95e38 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,6 +16,11 @@
 #include linux/phy.h
 #include linux/module.h
 
+#define RTL8201F_INSR  0x1e
+#define RTL8201F_PGSR  0x1f
+#define RTL8201F_INER  0x13
+#define RTL8201F_INER_MASK 0x3800
+
 #define RTL821x_PHYSR  0x11
 #define RTL821x_PHYSR_DUPLEX   0x2000
 #define RTL821x_PHYSR_SPEED0xc000
@@ -29,6 +34,15 @@ MODULE_DESCRIPTION(Realtek PHY driver);
 MODULE_AUTHOR(Johnson Leung);
 MODULE_LICENSE(GPL);
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_INSR);
+
+   return (err  0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +52,29 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err  0) ? err : 0;
 }
 
+static void rtl8201f_select_page(struct phy_device *phydev, int page)
+{
+   phy_write(phydev, RTL8201F_PGSR, page);
+}
+
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   rtl8201f_select_page(phydev, 7);
+
+   if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_INER, RTL8201F_INER_MASK |
+   phy_read(phydev, RTL8201F_INER));
+   else
+   err = phy_write(phydev, RTL8201F_INER, ~RTL8201F_INER_MASK 
+   phy_read(phydev, RTL8201F_INER));
+
+   rtl8201f_select_page(phydev, 0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -65,6 +102,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
 }
 
 static struct phy_driver realtek_drv[] = {
+   {   /* RTL8201F */
+   .phy_id = 0x001cc816,
+   .name   = RTL8201F 10/100Mbps Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl8201f_ack_interrupt,
+   .config_intr= rtl8201f_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
{   /* RTL8211B */
.phy_id = 0x001cc912,
.name   = RTL8211B Gigabit Ethernet,
@@ -107,6 +158,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 3/4] net: phy: realtek: add suspend/resume handlers for the RTL8211B

2013-05-10 Thread Jongsung Kim
Add compatible but missing generic PHY suspend/resume handlers for
the RTL8211B driver.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e95e38..6f0726a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -126,6 +126,8 @@ static struct phy_driver realtek_drv[] = {
.read_status= genphy_read_status,
.ack_interrupt  = rtl821x_ack_interrupt,
.config_intr= rtl8211b_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
},
{   /* RTL8211E */
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2 1/4] net: phy: realtek: simplify multiple drivers registration

2013-05-10 Thread Jongsung Kim
Use the phy_drivers_register and phy_drivers_unregister to
simplify registration and error handling. The two existing
phy_driver structures are converted to an array of the
structures to do this.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |   65 -
 1 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..f902107 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -64,50 +64,43 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
-/* RTL8211B */
-static struct phy_driver rtl8211b_driver = {
-   .phy_id = 0x001cc912,
-   .name   = RTL8211B Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211b_config_intr,
-   .driver = { .owner = THIS_MODULE,},
-};
-
-/* RTL8211E */
-static struct phy_driver rtl8211e_driver = {
-   .phy_id = 0x001cc915,
-   .name   = RTL8211E Gigabit Ethernet,
-   .phy_id_mask= 0x001f,
-   .features   = PHY_GBIT_FEATURES,
-   .flags  = PHY_HAS_INTERRUPT,
-   .config_aneg= genphy_config_aneg,
-   .read_status= genphy_read_status,
-   .ack_interrupt  = rtl821x_ack_interrupt,
-   .config_intr= rtl8211e_config_intr,
-   .suspend= genphy_suspend,
-   .resume = genphy_resume,
-   .driver = { .owner = THIS_MODULE,},
+static struct phy_driver realtek_drv[] = {
+   {   /* RTL8211B */
+   .phy_id = 0x001cc912,
+   .name   = RTL8211B Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211b_config_intr,
+   .driver = { .owner = THIS_MODULE,},
+   },
+   {   /* RTL8211E */
+   .phy_id = 0x001cc915,
+   .name   = RTL8211E Gigabit Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_GBIT_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl821x_ack_interrupt,
+   .config_intr= rtl8211e_config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   .driver = { .owner = THIS_MODULE,},
+   },
 };
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(rtl8211b_driver);
-   if (ret  0)
-   return -ENODEV;
-   return phy_driver_register(rtl8211e_driver);
+   return phy_drivers_register(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 static void __exit realtek_exit(void)
 {
-   phy_driver_unregister(rtl8211b_driver);
-   phy_driver_unregister(rtl8211e_driver);
+   phy_drivers_unregister(realtek_drv, ARRAY_SIZE(realtek_drv));
 }
 
 module_init(realtek_init);
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RESEND] tty: serial: amba-pl011: revise to use amba_rev macro

2013-05-10 Thread Jongsung Kim
This patch replaces the ugly bit-operations with the convenient
amba_rev macro from the get_fifosize_arm function. The type of
get_fifosize member function is also slightly changed to take
a pointer to the amba_device.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/tty/serial/amba-pl011.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8ab70a6..85cfe11 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -79,13 +79,12 @@ struct vendor_data {
booldma_threshold;
boolcts_event_workaround;
 
-   unsigned int (*get_fifosize)(unsigned int periphid);
+   unsigned int (*get_fifosize)(struct amba_device *dev);
 };
 
-static unsigned int get_fifosize_arm(unsigned int periphid)
+static unsigned int get_fifosize_arm(struct amba_device *dev)
 {
-   unsigned int rev = (periphid  20)  0xf;
-   return rev  3 ? 16 : 32;
+   return amba_rev(dev)  3 ? 16 : 32;
 }
 
 static struct vendor_data vendor_arm = {
@@ -98,7 +97,7 @@ static struct vendor_data vendor_arm = {
.get_fifosize   = get_fifosize_arm,
 };
 
-static unsigned int get_fifosize_st(unsigned int periphid)
+static unsigned int get_fifosize_st(struct amba_device *dev)
 {
return 64;
 }
@@ -2157,7 +2156,7 @@ static int pl011_probe(struct amba_device *dev, const 
struct amba_id *id)
uap-lcrh_rx = vendor-lcrh_rx;
uap-lcrh_tx = vendor-lcrh_tx;
uap-old_cr = 0;
-   uap-fifosize = vendor-get_fifosize(dev-periphid);
+   uap-fifosize = vendor-get_fifosize(dev);
uap-port.dev = dev-dev;
uap-port.mapbase = dev-res.start;
uap-port.membase = base;
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-09 Thread Jongsung Kim
Sergei Shtylyov  :
>> Agreed. I just thought it's better to make it similar to the 
>> rtl821x_ack_interrupt.
>
>Ah, then you may leave this code as is.
>
>> Then, may I make shorter the rtl821x_ack_interrupt as well as 
>> rtl8201f_ack_interrupt?
>
>In a separate patch, if you wish.
>

Thank you for your comment.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-09 Thread Jongsung Kim
Sergei Shtylyov sergei.shtyl...@cogentembedded.com :
 Agreed. I just thought it's better to make it similar to the 
 rtl821x_ack_interrupt.

Ah, then you may leave this code as is.

 Then, may I make shorter the rtl821x_ack_interrupt as well as 
 rtl8201f_ack_interrupt?

In a separate patch, if you wish.


Thank you for your comment.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
Sergei Shtylyov  :
> Removal of unused #define's is a matter of a separate cleanup patch...

Sorry. I won't touch them.

>> +static int rtl8201f_ack_interrupt(struct phy_device *phydev) {
>> +int err;
>> +
>> +err = phy_read(phydev, RTL8201F_ISR);
>
>This could be an initializer and so make the function shorter.
>

Agreed. I just thought it's better to make it similar to the
rtl821x_ack_interrupt. Then, may I make shorter the rtl821x_ack_interrupt as
well as rtl8201f_ack_interrupt?

>
> You haven't run this patch thru scripts/checkpatch.pl -- there should
be a space between *if* and (.
>

Sorry.. what a mistake..


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
Francois Romieu  :
> Your patch contains both "remove unused #define" and "support new
hardware"
> parts. I am not sure that the former is adequate for submission until
net-next opens.

I see. Sorry for trying touching them even without comment. I won't touch
them.

> static void rtl8201f_page_select(struct phy_device *phydev, int page) ?

Okay. Looks better..

>> +if(phy_driver_register(_driver) < 0)
> ^^ -> missing space.

What a shame!

> You may use an array of phy_driver for realtek_{init/exit}

Agreed.

--
Ueimor

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
This patch adds the minimal driver to manage the
Realtek RTL8201F 10/100Mbps Transceivers.

Signed-off-by: Jongsung Kim 
---
 drivers/net/phy/realtek.c |   60 +++-
 1 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..27847ca 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,9 +16,13 @@
 #include 
 #include 
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
+/* page 0 register 30 - interrupt indicators and SNR display register */
+#define RTL8201F_ISR   0x1e
+/* page 0 register 31 - page select register */
+#define RTL8201F_PSR   0x1f
+/* page 7 register 19 - interrupt, WOL enable, and LEDs function register */
+#define RTL8201F_IER   0x13
+
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -29,6 +33,15 @@ MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_ISR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +51,24 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   phy_write(phydev, RTL8201F_PSR, 0x0007);/* select page 7 */
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_IER, 0x3800 |
+   phy_read(phydev, RTL8201F_IER));
+   else
+   err = phy_write(phydev, RTL8201F_IER, ~0x3800 &
+   phy_read(phydev, RTL8201F_IER));
+
+   phy_write(phydev, RTL8201F_PSR, 0x);/* back to page 0 */
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -64,6 +95,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
+/* RTL8201F */
+static struct phy_driver rtl8201f_driver = {
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .driver = { .owner = THIS_MODULE,},
+};
+
 /* RTL8211B */
 static struct phy_driver rtl8211b_driver = {
.phy_id = 0x001cc912,
@@ -96,16 +141,16 @@ static struct phy_driver rtl8211e_driver = {
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(_driver);
-   if (ret < 0)
+   if(phy_driver_register(_driver) < 0)
+   return -ENODEV;
+   if(phy_driver_register(_driver) < 0)
return -ENODEV;
return phy_driver_register(_driver);
 }
 
 static void __exit realtek_exit(void)
 {
+   phy_driver_unregister(_driver);
phy_driver_unregister(_driver);
phy_driver_unregister(_driver);
 }
@@ -114,6 +159,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
This patch adds the minimal driver to manage the
Realtek RTL8201F 10/100Mbps Transceivers.

Signed-off-by: Jongsung Kim neidhard@lge.com
---
 drivers/net/phy/realtek.c |   60 +++-
 1 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 8e7af83..27847ca 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -16,9 +16,13 @@
 #include linux/phy.h
 #include linux/module.h
 
-#define RTL821x_PHYSR  0x11
-#define RTL821x_PHYSR_DUPLEX   0x2000
-#define RTL821x_PHYSR_SPEED0xc000
+/* page 0 register 30 - interrupt indicators and SNR display register */
+#define RTL8201F_ISR   0x1e
+/* page 0 register 31 - page select register */
+#define RTL8201F_PSR   0x1f
+/* page 7 register 19 - interrupt, WOL enable, and LEDs function register */
+#define RTL8201F_IER   0x13
+
 #define RTL821x_INER   0x12
 #define RTL821x_INER_INIT  0x6400
 #define RTL821x_INSR   0x13
@@ -29,6 +33,15 @@ MODULE_DESCRIPTION(Realtek PHY driver);
 MODULE_AUTHOR(Johnson Leung);
 MODULE_LICENSE(GPL);
 
+static int rtl8201f_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_ISR);
+
+   return (err  0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -38,6 +51,24 @@ static int rtl821x_ack_interrupt(struct phy_device *phydev)
return (err  0) ? err : 0;
 }
 
+static int rtl8201f_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   phy_write(phydev, RTL8201F_PSR, 0x0007);/* select page 7 */
+
+   if (phydev-interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_IER, 0x3800 |
+   phy_read(phydev, RTL8201F_IER));
+   else
+   err = phy_write(phydev, RTL8201F_IER, ~0x3800 
+   phy_read(phydev, RTL8201F_IER));
+
+   phy_write(phydev, RTL8201F_PSR, 0x);/* back to page 0 */
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -64,6 +95,20 @@ static int rtl8211e_config_intr(struct phy_device *phydev)
return err;
 }
 
+/* RTL8201F */
+static struct phy_driver rtl8201f_driver = {
+   .phy_id = 0x001cc816,
+   .name   = RTL8201F 10/100Mbps Ethernet,
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= genphy_config_aneg,
+   .read_status= genphy_read_status,
+   .ack_interrupt  = rtl8201f_ack_interrupt,
+   .config_intr= rtl8201f_config_intr,
+   .driver = { .owner = THIS_MODULE,},
+};
+
 /* RTL8211B */
 static struct phy_driver rtl8211b_driver = {
.phy_id = 0x001cc912,
@@ -96,16 +141,16 @@ static struct phy_driver rtl8211e_driver = {
 
 static int __init realtek_init(void)
 {
-   int ret;
-
-   ret = phy_driver_register(rtl8211b_driver);
-   if (ret  0)
+   if(phy_driver_register(rtl8201f_driver)  0)
+   return -ENODEV;
+   if(phy_driver_register(rtl8211b_driver)  0)
return -ENODEV;
return phy_driver_register(rtl8211e_driver);
 }
 
 static void __exit realtek_exit(void)
 {
+   phy_driver_unregister(rtl8201f_driver);
phy_driver_unregister(rtl8211b_driver);
phy_driver_unregister(rtl8211e_driver);
 }
@@ -114,6 +159,7 @@ module_init(realtek_init);
 module_exit(realtek_exit);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc915, 0x001f },
{ }
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
Francois Romieu rom...@fr.zoreil.com :
 Your patch contains both remove unused #define and support new
hardware
 parts. I am not sure that the former is adequate for submission until
net-next opens.

I see. Sorry for trying touching them even without comment. I won't touch
them.

 static void rtl8201f_page_select(struct phy_device *phydev, int page) ?

Okay. Looks better..

 +if(phy_driver_register(rtl8201f_driver)  0)
 ^^ - missing space.

What a shame!

 You may use an array of phy_driver for realtek_{init/exit}

Agreed.

--
Ueimor

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: net: phy: realtek: add rtl8201f driver

2013-05-08 Thread Jongsung Kim
Sergei Shtylyov sergei.shtyl...@cogentembedded.com :
 Removal of unused #define's is a matter of a separate cleanup patch...

Sorry. I won't touch them.

 +static int rtl8201f_ack_interrupt(struct phy_device *phydev) {
 +int err;
 +
 +err = phy_read(phydev, RTL8201F_ISR);

This could be an initializer and so make the function shorter.


Agreed. I just thought it's better to make it similar to the
rtl821x_ack_interrupt. Then, may I make shorter the rtl821x_ack_interrupt as
well as rtl8201f_ack_interrupt?


 You haven't run this patch thru scripts/checkpatch.pl -- there should
be a space between *if* and (.


Sorry.. what a mistake..


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ARM: PL011: revise to use amba_rev macro

2013-05-03 Thread Jongsung Kim
This patch replaces the ugly bit-operations with the convenient
amba_rev macro from the get_fifosize_arm function. The type of
get_fifosize member function is also slightly changed to take
a pointer to the amba_device.

Signed-off-by: Jongsung Kim 
---
 drivers/tty/serial/amba-pl011.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index b2e9e17..72b57f9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -79,13 +79,12 @@ struct vendor_data {
booldma_threshold;
boolcts_event_workaround;
 
-   unsigned int (*get_fifosize)(unsigned int periphid);
+   unsigned int (*get_fifosize)(struct amba_device *dev);
 };
 
-static unsigned int get_fifosize_arm(unsigned int periphid)
+static unsigned int get_fifosize_arm(struct amba_device *dev)
 {
-   unsigned int rev = (periphid >> 20) & 0xf;
-   return rev < 3 ? 16 : 32;
+   return amba_rev(dev) < 3 ? 16 : 32;
 }
 
 static struct vendor_data vendor_arm = {
@@ -98,7 +97,7 @@ static struct vendor_data vendor_arm = {
.get_fifosize   = get_fifosize_arm,
 };
 
-static unsigned int get_fifosize_st(unsigned int periphid)
+static unsigned int get_fifosize_st(struct amba_device *dev)
 {
return 64;
 }
@@ -2145,7 +2144,7 @@ static int pl011_probe(struct amba_device *dev, const 
struct amba_id *id)
uap->lcrh_rx = vendor->lcrh_rx;
uap->lcrh_tx = vendor->lcrh_tx;
uap->old_cr = 0;
-   uap->fifosize = vendor->get_fifosize(dev->periphid);
+   uap->fifosize = vendor->get_fifosize(dev);
uap->port.dev = >dev;
uap->port.mapbase = dev->res.start;
uap->port.membase = base;
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


  1   2   >