Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-29 Thread Kishon Vijay Abraham I 


On Tuesday 28 October 2014 12:02 AM, Sebastian Hesselbarth wrote:
> On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:
>> On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
>>> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
 On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
>> On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
>>> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
>>> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
>>> is different. Prepare the driver for BG2 support by moving the phy_base
>>> into private driver data.
>>>
>>> Acked-by: Antoine Ténart 
>>> Signed-off-by: Sebastian Hesselbarth 
> ...
>>> ---
>>>   drivers/phy/phy-berlin-sata.c | 42
>>> --
>>>   1 file changed, 28 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/phy/phy-berlin-sata.c
>>> b/drivers/phy/phy-berlin-sata.c
>>> index 69ced52d72aa..9682b0f66177 100644
>>> --- a/drivers/phy/phy-berlin-sata.c
>>> +++ b/drivers/phy/phy-berlin-sata.c
>>> @@ -30,7 +30,7 @@
>>>   #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
>>>   #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)
>>>
>>> -#define PHY_BASE0x200
>>> +#define BG2Q_PHY_BASE0x200
> [...]
>>> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
>>> +
>>> +static const struct of_device_id phy_berlin_sata_of_match[] = {
>>> +{
>>> +.compatible = "marvell,berlin2q-sata-phy",
>>> +.data = _sata_phy_base,
>>
>> Can't the base directly come from dt?
>
> You are suggesting a "marvell,phy-base-address" property, right?
> I have no strong opinion about it, I accept your call (or DT maintainer
> ones).

 I still have the DT patches for BG2Q queued up for v3.19 (I missed the
 arm-soc merge window for v3.18). That means, there has been no release
 with the phy binding used and I can rework a little more.

 Can you please confirm that you want a DT property for the phy base 
 address,
 e.g. marvell,phy-base-address = <{0x200,0x80}> ?

 If so, I'd also rename the compatible from berlin2q-sata-phy to more
 generic berlin-sata-phy.
>>>
>>> I think what Kishon is asking, is why this 0x200 offset isn't already on
>>> reg. so that instead of, e.g.:
>>>
>>> reg = <0x4000 0x1000>;
>>>
>>> you would have:
>>>
>>> reg = <0x4200 0x1000>;
>>
>> I had something similar to what Sebastian suggested in mind. I think phy_base
>> is used for a different reason and can't be directly used in 'reg'.
> 
> Kishon,
> 
> thanks for the clarification. While the extra marvell,phy-base-address
> property basically works and I agree with it, I may have some
> _potential_ draw-backs:
> 
> The Marvell BSP code (which I have no clue _why_ it does what it does
> or if it is required) has some magic writes to "improve" serial signal
> quality. I left them out as my HDD was detected with and without them.
> 
> Now, if we find that they are required, we have to find a way to make
> the PHY driver know about the PHY revision. We'd usually add a
> different compatible and deal with it accordingly.
> 
> So, not adding the compatible now _may_ just postpone a follow-up patch
> for the different PHY setup of BG2 and render the new phy_base property
> basically useless.
> 
> If you are just unhappy with the "static u32 bg2q_sata_phy_base"
> assigned to of_device_id.data, I can convert that to Felipe's proposal.

Either way is fine with me.

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-29 Thread Kishon Vijay Abraham I 


On Tuesday 28 October 2014 12:02 AM, Sebastian Hesselbarth wrote:
 On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:
 On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
 On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
 On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
 On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
 On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
 Currently, Berlin SATA PHY driver assumes PHY_BASE address being
 constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
 is different. Prepare the driver for BG2 support by moving the phy_base
 into private driver data.

 Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
 Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 ...
 ---
   drivers/phy/phy-berlin-sata.c | 42
 --
   1 file changed, 28 insertions(+), 14 deletions(-)

 diff --git a/drivers/phy/phy-berlin-sata.c
 b/drivers/phy/phy-berlin-sata.c
 index 69ced52d72aa..9682b0f66177 100644
 --- a/drivers/phy/phy-berlin-sata.c
 +++ b/drivers/phy/phy-berlin-sata.c
 @@ -30,7 +30,7 @@
   #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
   #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

 -#define PHY_BASE0x200
 +#define BG2Q_PHY_BASE0x200
 [...]
 +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
 +
 +static const struct of_device_id phy_berlin_sata_of_match[] = {
 +{
 +.compatible = marvell,berlin2q-sata-phy,
 +.data = bg2q_sata_phy_base,

 Can't the base directly come from dt?

 You are suggesting a marvell,phy-base-address property, right?
 I have no strong opinion about it, I accept your call (or DT maintainer
 ones).

 I still have the DT patches for BG2Q queued up for v3.19 (I missed the
 arm-soc merge window for v3.18). That means, there has been no release
 with the phy binding used and I can rework a little more.

 Can you please confirm that you want a DT property for the phy base 
 address,
 e.g. marvell,phy-base-address = {0x200,0x80} ?

 If so, I'd also rename the compatible from berlin2q-sata-phy to more
 generic berlin-sata-phy.

 I think what Kishon is asking, is why this 0x200 offset isn't already on
 reg. so that instead of, e.g.:

 reg = 0x4000 0x1000;

 you would have:

 reg = 0x4200 0x1000;

 I had something similar to what Sebastian suggested in mind. I think phy_base
 is used for a different reason and can't be directly used in 'reg'.
 
 Kishon,
 
 thanks for the clarification. While the extra marvell,phy-base-address
 property basically works and I agree with it, I may have some
 _potential_ draw-backs:
 
 The Marvell BSP code (which I have no clue _why_ it does what it does
 or if it is required) has some magic writes to improve serial signal
 quality. I left them out as my HDD was detected with and without them.
 
 Now, if we find that they are required, we have to find a way to make
 the PHY driver know about the PHY revision. We'd usually add a
 different compatible and deal with it accordingly.
 
 So, not adding the compatible now _may_ just postpone a follow-up patch
 for the different PHY setup of BG2 and render the new phy_base property
 basically useless.
 
 If you are just unhappy with the static u32 bg2q_sata_phy_base
 assigned to of_device_id.data, I can convert that to Felipe's proposal.

Either way is fine with me.

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-27 Thread Sebastian Hesselbarth 

On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:

On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart 
Signed-off-by: Sebastian Hesselbarth 

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = "marvell,berlin2q-sata-phy",
+.data = _sata_phy_base,


Can't the base directly come from dt?


You are suggesting a "marvell,phy-base-address" property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base address,
e.g. marvell,phy-base-address = <{0x200,0x80}> ?

If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.


I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = <0x4000 0x1000>;

you would have:

reg = <0x4200 0x1000>;


I had something similar to what Sebastian suggested in mind. I think phy_base
is used for a different reason and can't be directly used in 'reg'.


Kishon,

thanks for the clarification. While the extra marvell,phy-base-address
property basically works and I agree with it, I may have some
_potential_ draw-backs:

The Marvell BSP code (which I have no clue _why_ it does what it does
or if it is required) has some magic writes to "improve" serial signal
quality. I left them out as my HDD was detected with and without them.

Now, if we find that they are required, we have to find a way to make
the PHY driver know about the PHY revision. We'd usually add a
different compatible and deal with it accordingly.

So, not adding the compatible now _may_ just postpone a follow-up patch
for the different PHY setup of BG2 and render the new phy_base property
basically useless.

If you are just unhappy with the "static u32 bg2q_sata_phy_base"
assigned to of_device_id.data, I can convert that to Felipe's proposal.

Sebastian
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-27 Thread Kishon Vijay Abraham I 
Hi,

On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
> Hi,
> 
> On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
>> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
>>> On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
 On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
> is different. Prepare the driver for BG2 support by moving the phy_base
> into private driver data.
>
> Acked-by: Antoine Ténart 
> Signed-off-by: Sebastian Hesselbarth 
>>> ...
> ---
>  drivers/phy/phy-berlin-sata.c | 42
> --
>  1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/phy/phy-berlin-sata.c
> b/drivers/phy/phy-berlin-sata.c
> index 69ced52d72aa..9682b0f66177 100644
> --- a/drivers/phy/phy-berlin-sata.c
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -30,7 +30,7 @@
>  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
>  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)
>
> -#define PHY_BASE0x200
> +#define BG2Q_PHY_BASE0x200
>>> [...]
> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> +{
> +.compatible = "marvell,berlin2q-sata-phy",
> +.data = _sata_phy_base,

 Can't the base directly come from dt?
>>>
>>> You are suggesting a "marvell,phy-base-address" property, right?
>>> I have no strong opinion about it, I accept your call (or DT maintainer
>>> ones).
>>
>> Kishon,
>>
>> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
>> arm-soc merge window for v3.18). That means, there has been no release
>> with the phy binding used and I can rework a little more.
>>
>> Can you please confirm that you want a DT property for the phy base address,
>> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
>>
>> If so, I'd also rename the compatible from berlin2q-sata-phy to more
>> generic berlin-sata-phy.
> 
> I think what Kishon is asking, is why this 0x200 offset isn't already on
> reg. so that instead of, e.g.:
> 
>   reg = <0x4000 0x1000>;
> 
> you would have:
> 
>   reg = <0x4200 0x1000>;

I had something similar to what Sebastian suggested in mind. I think phy_base
is used for a different reason and can't be directly used in 'reg'.

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-27 Thread Kishon Vijay Abraham I 
Hi,

On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:
 Hi,
 
 On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
 On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
 On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
 On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
 Currently, Berlin SATA PHY driver assumes PHY_BASE address being
 constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
 is different. Prepare the driver for BG2 support by moving the phy_base
 into private driver data.

 Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
 Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 ...
 ---
  drivers/phy/phy-berlin-sata.c | 42
 --
  1 file changed, 28 insertions(+), 14 deletions(-)

 diff --git a/drivers/phy/phy-berlin-sata.c
 b/drivers/phy/phy-berlin-sata.c
 index 69ced52d72aa..9682b0f66177 100644
 --- a/drivers/phy/phy-berlin-sata.c
 +++ b/drivers/phy/phy-berlin-sata.c
 @@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

 -#define PHY_BASE0x200
 +#define BG2Q_PHY_BASE0x200
 [...]
 +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
 +
 +static const struct of_device_id phy_berlin_sata_of_match[] = {
 +{
 +.compatible = marvell,berlin2q-sata-phy,
 +.data = bg2q_sata_phy_base,

 Can't the base directly come from dt?

 You are suggesting a marvell,phy-base-address property, right?
 I have no strong opinion about it, I accept your call (or DT maintainer
 ones).

 Kishon,

 I still have the DT patches for BG2Q queued up for v3.19 (I missed the
 arm-soc merge window for v3.18). That means, there has been no release
 with the phy binding used and I can rework a little more.

 Can you please confirm that you want a DT property for the phy base address,
 e.g. marvell,phy-base-address = {0x200,0x80} ?

 If so, I'd also rename the compatible from berlin2q-sata-phy to more
 generic berlin-sata-phy.
 
 I think what Kishon is asking, is why this 0x200 offset isn't already on
 reg. so that instead of, e.g.:
 
   reg = 0x4000 0x1000;
 
 you would have:
 
   reg = 0x4200 0x1000;

I had something similar to what Sebastian suggested in mind. I think phy_base
is used for a different reason and can't be directly used in 'reg'.

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-27 Thread Sebastian Hesselbarth 

On 10/27/2014 01:27 PM, Kishon Vijay Abraham I wrote:

On Saturday 25 October 2014 01:55 AM, Felipe Balbi wrote:

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = marvell,berlin2q-sata-phy,
+.data = bg2q_sata_phy_base,


Can't the base directly come from dt?


You are suggesting a marvell,phy-base-address property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base address,
e.g. marvell,phy-base-address = {0x200,0x80} ?

If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.


I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = 0x4000 0x1000;

you would have:

reg = 0x4200 0x1000;


I had something similar to what Sebastian suggested in mind. I think phy_base
is used for a different reason and can't be directly used in 'reg'.


Kishon,

thanks for the clarification. While the extra marvell,phy-base-address
property basically works and I agree with it, I may have some
_potential_ draw-backs:

The Marvell BSP code (which I have no clue _why_ it does what it does
or if it is required) has some magic writes to improve serial signal
quality. I left them out as my HDD was detected with and without them.

Now, if we find that they are required, we have to find a way to make
the PHY driver know about the PHY revision. We'd usually add a
different compatible and deal with it accordingly.

So, not adding the compatible now _may_ just postpone a follow-up patch
for the different PHY setup of BG2 and render the new phy_base property
basically useless.

If you are just unhappy with the static u32 bg2q_sata_phy_base
assigned to of_device_id.data, I can convert that to Felipe's proposal.

Sebastian
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Sebastian Hesselbarth 

On 24.10.2014 22:25, Felipe Balbi wrote:

Hi,

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart 
Signed-off-by: Sebastian Hesselbarth 

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = "marvell,berlin2q-sata-phy",
+.data = _sata_phy_base,


Can't the base directly come from dt?


You are suggesting a "marvell,phy-base-address" property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


Kishon,

I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base address,
e.g. marvell,phy-base-address = <{0x200,0x80}> ?

If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.


I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = <0x4000 0x1000>;

you would have:

reg = <0x4200 0x1000>;


Because it is the PHY_BASE offset within the _indirect_ register
access trough AHCI registers.


then everybody's happy. It's unfortunate, however, that we already
shipped DT sources with the bogus (?) reg property and now we have to
support that broken binding. My suggestion would be to add a new
compatible which comes with proper reg property and still add that weird
phy_base for the old compatible, so that:


Nope, the reg property is correct and describes the (vendor-specific)
AHCI registers that are used for indirect access to PHY registers. When
writing to PHY registers, BG2 and BG2Q are different with respect to
the PHY_BASE address offset that has to be passed to indirect address
register.


if (of_device_is_compatible(node, "marvell,berlin2q-sata-phy"))
phy->phy_base = PHY_BASE;

then, if new compatible comes with proper 'reg', phy->phy_base would be
zero and everything should still work. How does this sound ?


The above is basically equivalent to the node match that I added with
this patch series. I can, of course, use above compatible match instead
of passing the phy_base in the of_device_id's .data.

Sebastian

--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Felipe Balbi 
Hi,

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
> On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
> >On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
> >>On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
> >>>Currently, Berlin SATA PHY driver assumes PHY_BASE address being
> >>>constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
> >>>is different. Prepare the driver for BG2 support by moving the phy_base
> >>>into private driver data.
> >>>
> >>>Acked-by: Antoine Ténart 
> >>>Signed-off-by: Sebastian Hesselbarth 
> >...
> >>>---
> >>>  drivers/phy/phy-berlin-sata.c | 42
> >>>--
> >>>  1 file changed, 28 insertions(+), 14 deletions(-)
> >>>
> >>>diff --git a/drivers/phy/phy-berlin-sata.c
> >>>b/drivers/phy/phy-berlin-sata.c
> >>>index 69ced52d72aa..9682b0f66177 100644
> >>>--- a/drivers/phy/phy-berlin-sata.c
> >>>+++ b/drivers/phy/phy-berlin-sata.c
> >>>@@ -30,7 +30,7 @@
> >>>  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
> >>>  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)
> >>>
> >>>-#define PHY_BASE0x200
> >>>+#define BG2Q_PHY_BASE0x200
> >[...]
> >>>+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
> >>>+
> >>>+static const struct of_device_id phy_berlin_sata_of_match[] = {
> >>>+{
> >>>+.compatible = "marvell,berlin2q-sata-phy",
> >>>+.data = _sata_phy_base,
> >>
> >>Can't the base directly come from dt?
> >
> >You are suggesting a "marvell,phy-base-address" property, right?
> >I have no strong opinion about it, I accept your call (or DT maintainer
> >ones).
> 
> Kishon,
> 
> I still have the DT patches for BG2Q queued up for v3.19 (I missed the
> arm-soc merge window for v3.18). That means, there has been no release
> with the phy binding used and I can rework a little more.
> 
> Can you please confirm that you want a DT property for the phy base address,
> e.g. marvell,phy-base-address = <{0x200,0x80}> ?
> 
> If so, I'd also rename the compatible from berlin2q-sata-phy to more
> generic berlin-sata-phy.

I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = <0x4000 0x1000>;

you would have:

reg = <0x4200 0x1000>;

then everybody's happy. It's unfortunate, however, that we already
shipped DT sources with the bogus (?) reg property and now we have to
support that broken binding. My suggestion would be to add a new
compatible which comes with proper reg property and still add that weird
phy_base for the old compatible, so that:

if (of_device_is_compatible(node, "marvell,berlin2q-sata-phy"))
phy->phy_base = PHY_BASE;

then, if new compatible comes with proper 'reg', phy->phy_base would be
zero and everything should still work. How does this sound ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Sebastian Hesselbarth 

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart 
Signed-off-by: Sebastian Hesselbarth 

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2) << 16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = "marvell,berlin2q-sata-phy",
+.data = _sata_phy_base,


Can't the base directly come from dt?


You are suggesting a "marvell,phy-base-address" property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


Kishon,

I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base 
address, e.g. marvell,phy-base-address = <{0x200,0x80}> ?


If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.

Sebastian
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Sebastian Hesselbarth 

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = marvell,berlin2q-sata-phy,
+.data = bg2q_sata_phy_base,


Can't the base directly come from dt?


You are suggesting a marvell,phy-base-address property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


Kishon,

I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base 
address, e.g. marvell,phy-base-address = {0x200,0x80} ?


If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.

Sebastian
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Felipe Balbi 
Hi,

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:
 On 21.10.2014 11:40, Sebastian Hesselbarth wrote:
 On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:
 On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
 Currently, Berlin SATA PHY driver assumes PHY_BASE address being
 constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
 is different. Prepare the driver for BG2 support by moving the phy_base
 into private driver data.
 
 Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
 Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 ...
 ---
   drivers/phy/phy-berlin-sata.c | 42
 --
   1 file changed, 28 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/phy/phy-berlin-sata.c
 b/drivers/phy/phy-berlin-sata.c
 index 69ced52d72aa..9682b0f66177 100644
 --- a/drivers/phy/phy-berlin-sata.c
 +++ b/drivers/phy/phy-berlin-sata.c
 @@ -30,7 +30,7 @@
   #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
   #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)
 
 -#define PHY_BASE0x200
 +#define BG2Q_PHY_BASE0x200
 [...]
 +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
 +
 +static const struct of_device_id phy_berlin_sata_of_match[] = {
 +{
 +.compatible = marvell,berlin2q-sata-phy,
 +.data = bg2q_sata_phy_base,
 
 Can't the base directly come from dt?
 
 You are suggesting a marvell,phy-base-address property, right?
 I have no strong opinion about it, I accept your call (or DT maintainer
 ones).
 
 Kishon,
 
 I still have the DT patches for BG2Q queued up for v3.19 (I missed the
 arm-soc merge window for v3.18). That means, there has been no release
 with the phy binding used and I can rework a little more.
 
 Can you please confirm that you want a DT property for the phy base address,
 e.g. marvell,phy-base-address = {0x200,0x80} ?
 
 If so, I'd also rename the compatible from berlin2q-sata-phy to more
 generic berlin-sata-phy.

I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = 0x4000 0x1000;

you would have:

reg = 0x4200 0x1000;

then everybody's happy. It's unfortunate, however, that we already
shipped DT sources with the bogus (?) reg property and now we have to
support that broken binding. My suggestion would be to add a new
compatible which comes with proper reg property and still add that weird
phy_base for the old compatible, so that:

if (of_device_is_compatible(node, marvell,berlin2q-sata-phy))
phy-phy_base = PHY_BASE;

then, if new compatible comes with proper 'reg', phy-phy_base would be
zero and everything should still work. How does this sound ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-24 Thread Sebastian Hesselbarth 

On 24.10.2014 22:25, Felipe Balbi wrote:

Hi,

On Fri, Oct 24, 2014 at 10:14:55PM +0200, Sebastian Hesselbarth wrote:

On 21.10.2014 11:40, Sebastian Hesselbarth wrote:

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com

...

---
  drivers/phy/phy-berlin-sata.c | 42
--
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c
b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128(BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

-#define PHY_BASE0x200
+#define BG2Q_PHY_BASE0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+{
+.compatible = marvell,berlin2q-sata-phy,
+.data = bg2q_sata_phy_base,


Can't the base directly come from dt?


You are suggesting a marvell,phy-base-address property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).


Kishon,

I still have the DT patches for BG2Q queued up for v3.19 (I missed the
arm-soc merge window for v3.18). That means, there has been no release
with the phy binding used and I can rework a little more.

Can you please confirm that you want a DT property for the phy base address,
e.g. marvell,phy-base-address = {0x200,0x80} ?

If so, I'd also rename the compatible from berlin2q-sata-phy to more
generic berlin-sata-phy.


I think what Kishon is asking, is why this 0x200 offset isn't already on
reg. so that instead of, e.g.:

reg = 0x4000 0x1000;

you would have:

reg = 0x4200 0x1000;


Because it is the PHY_BASE offset within the _indirect_ register
access trough AHCI registers.


then everybody's happy. It's unfortunate, however, that we already
shipped DT sources with the bogus (?) reg property and now we have to
support that broken binding. My suggestion would be to add a new
compatible which comes with proper reg property and still add that weird
phy_base for the old compatible, so that:


Nope, the reg property is correct and describes the (vendor-specific)
AHCI registers that are used for indirect access to PHY registers. When
writing to PHY registers, BG2 and BG2Q are different with respect to
the PHY_BASE address offset that has to be passed to indirect address
register.


if (of_device_is_compatible(node, marvell,berlin2q-sata-phy))
phy-phy_base = PHY_BASE;

then, if new compatible comes with proper 'reg', phy-phy_base would be
zero and everything should still work. How does this sound ?


The above is basically equivalent to the node match that I added with
this patch series. I can, of course, use above compatible match instead
of passing the phy_base in the of_device_id's .data.

Sebastian

--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-21 Thread Sebastian Hesselbarth 

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart 
Signed-off-by: Sebastian Hesselbarth 

...

---
  drivers/phy/phy-berlin-sata.c | 42 --
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128   (BIT(2) << 16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2) << 19)

-#define PHY_BASE   0x200
+#define BG2Q_PHY_BASE  0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+   {
+   .compatible = "marvell,berlin2q-sata-phy",
+   .data = _sata_phy_base,


Can't the base directly come from dt?


You are suggesting a "marvell,phy-base-address" property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).

Sebastian
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-21 Thread Kishon Vijay Abraham I 


On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
> Currently, Berlin SATA PHY driver assumes PHY_BASE address being
> constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
> is different. Prepare the driver for BG2 support by moving the phy_base
> into private driver data.
> 
> Acked-by: Antoine Ténart 
> Signed-off-by: Sebastian Hesselbarth 
> ---
> Cc: Kishon Vijay Abraham I  
> Cc: "Antoine Ténart" 
> Cc: devicet...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org 
> ---
>  drivers/phy/phy-berlin-sata.c | 42 --
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> index 69ced52d72aa..9682b0f66177 100644
> --- a/drivers/phy/phy-berlin-sata.c
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -30,7 +30,7 @@
>  #define MBUS_WRITE_REQUEST_SIZE_128  (BIT(2) << 16)
>  #define MBUS_READ_REQUEST_SIZE_128   (BIT(2) << 19)
>  
> -#define PHY_BASE 0x200
> +#define BG2Q_PHY_BASE0x200
>  
>  /* register 0x01 */
>  #define REF_FREF_SEL_25  BIT(0)
> @@ -61,15 +61,16 @@ struct phy_berlin_priv {
>   struct clk  *clk;
>   struct phy_berlin_desc  **phys;
>   unsignednphys;
> + u32 phy_base;
>  };
>  
> -static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 
> reg,
> -u32 mask, u32 val)
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg,
> +u32 phy_base, u32 reg, u32 mask, u32 val)
>  {
>   u32 regval;
>  
>   /* select register */
> - writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> + writel(phy_base + reg, ctrl_reg + PORT_VSR_ADDR);
>  
>   /* set bits */
>   regval = readl(ctrl_reg + PORT_VSR_DATA);
> @@ -103,17 +104,20 @@ static int phy_berlin_sata_power_on(struct phy *phy)
>   writel(regval, priv->base + HOST_VSA_DATA);
>  
>   /* set PHY mode and ref freq to 25 MHz */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> - REF_FREF_SEL_25 | PHY_MODE_SATA);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x01,
> + 0x00ff, REF_FREF_SEL_25 | PHY_MODE_SATA);
>  
>   /* set PHY up to 6 Gbps */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x25,
> + 0x0c00, PHY_GEN_MAX_6_0);
>  
>   /* set 40 bits width */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x23,
> + 0x0c00, DATA_BIT_WIDTH_40);
>  
>   /* use max pll rate */
> - phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> + phy_berlin_sata_reg_setbits(ctrl_reg, priv->phy_base, 0x02,
> + 0x, USE_MAX_PLL_RATE);
>  
>   /* set Gen3 controller speed */
>   regval = readl(ctrl_reg + PORT_SCR_CTL);
> @@ -182,9 +186,22 @@ static u32 phy_berlin_power_down_bits[] = {
>   POWER_DOWN_PHY1,
>  };
>  
> +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> + {
> + .compatible = "marvell,berlin2q-sata-phy",
> + .data = _sata_phy_base,

Can't the base directly come from dt?

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-21 Thread Kishon Vijay Abraham I 


On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:
 Currently, Berlin SATA PHY driver assumes PHY_BASE address being
 constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
 is different. Prepare the driver for BG2 support by moving the phy_base
 into private driver data.
 
 Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
 Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
 ---
 Cc: Kishon Vijay Abraham I kis...@ti.com 
 Cc: Antoine Ténart antoine.ten...@free-electrons.com
 Cc: devicet...@vger.kernel.org
 Cc: linux-arm-ker...@lists.infradead.org
 Cc: linux-kernel@vger.kernel.org 
 ---
  drivers/phy/phy-berlin-sata.c | 42 --
  1 file changed, 28 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
 index 69ced52d72aa..9682b0f66177 100644
 --- a/drivers/phy/phy-berlin-sata.c
 +++ b/drivers/phy/phy-berlin-sata.c
 @@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128  (BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128   (BIT(2)  19)
  
 -#define PHY_BASE 0x200
 +#define BG2Q_PHY_BASE0x200
  
  /* register 0x01 */
  #define REF_FREF_SEL_25  BIT(0)
 @@ -61,15 +61,16 @@ struct phy_berlin_priv {
   struct clk  *clk;
   struct phy_berlin_desc  **phys;
   unsignednphys;
 + u32 phy_base;
  };
  
 -static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 
 reg,
 -u32 mask, u32 val)
 +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg,
 +u32 phy_base, u32 reg, u32 mask, u32 val)
  {
   u32 regval;
  
   /* select register */
 - writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
 + writel(phy_base + reg, ctrl_reg + PORT_VSR_ADDR);
  
   /* set bits */
   regval = readl(ctrl_reg + PORT_VSR_DATA);
 @@ -103,17 +104,20 @@ static int phy_berlin_sata_power_on(struct phy *phy)
   writel(regval, priv-base + HOST_VSA_DATA);
  
   /* set PHY mode and ref freq to 25 MHz */
 - phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
 - REF_FREF_SEL_25 | PHY_MODE_SATA);
 + phy_berlin_sata_reg_setbits(ctrl_reg, priv-phy_base, 0x01,
 + 0x00ff, REF_FREF_SEL_25 | PHY_MODE_SATA);
  
   /* set PHY up to 6 Gbps */
 - phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
 + phy_berlin_sata_reg_setbits(ctrl_reg, priv-phy_base, 0x25,
 + 0x0c00, PHY_GEN_MAX_6_0);
  
   /* set 40 bits width */
 - phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
 + phy_berlin_sata_reg_setbits(ctrl_reg, priv-phy_base, 0x23,
 + 0x0c00, DATA_BIT_WIDTH_40);
  
   /* use max pll rate */
 - phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
 + phy_berlin_sata_reg_setbits(ctrl_reg, priv-phy_base, 0x02,
 + 0x, USE_MAX_PLL_RATE);
  
   /* set Gen3 controller speed */
   regval = readl(ctrl_reg + PORT_SCR_CTL);
 @@ -182,9 +186,22 @@ static u32 phy_berlin_power_down_bits[] = {
   POWER_DOWN_PHY1,
  };
  
 +static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
 +
 +static const struct of_device_id phy_berlin_sata_of_match[] = {
 + {
 + .compatible = marvell,berlin2q-sata-phy,
 + .data = bg2q_sata_phy_base,

Can't the base directly come from dt?

Thanks
Kishon
--
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 1/5] phy: berlin-sata: Move PHY_BASE into private data struct

2014-10-21 Thread Sebastian Hesselbarth 

On 10/21/2014 11:33 AM, Kishon Vijay Abraham I wrote:

On Tuesday 21 October 2014 02:37 PM, Sebastian Hesselbarth wrote:

Currently, Berlin SATA PHY driver assumes PHY_BASE address being
constant. While this PHY_BASE is correct for BG2Q, older BG2 PHY_BASE
is different. Prepare the driver for BG2 support by moving the phy_base
into private driver data.

Acked-by: Antoine Ténart antoine.ten...@free-electrons.com
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com

...

---
  drivers/phy/phy-berlin-sata.c | 42 --
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
index 69ced52d72aa..9682b0f66177 100644
--- a/drivers/phy/phy-berlin-sata.c
+++ b/drivers/phy/phy-berlin-sata.c
@@ -30,7 +30,7 @@
  #define MBUS_WRITE_REQUEST_SIZE_128   (BIT(2)  16)
  #define MBUS_READ_REQUEST_SIZE_128(BIT(2)  19)

-#define PHY_BASE   0x200
+#define BG2Q_PHY_BASE  0x200

[...]

+static u32 bg2q_sata_phy_base = BG2Q_PHY_BASE;
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+   {
+   .compatible = marvell,berlin2q-sata-phy,
+   .data = bg2q_sata_phy_base,


Can't the base directly come from dt?


You are suggesting a marvell,phy-base-address property, right?
I have no strong opinion about it, I accept your call (or DT maintainer
ones).

Sebastian
--
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/