diff mbox

[2/2] sata_rcar: Add R-Car Gen2 SATA support

Message ID 1381765354-28440-3-git-send-email-valentine.barshak@cogentembedded.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Valentine Barshak Oct. 14, 2013, 3:42 p.m. UTC
R-Car Gen2 SoCs have a different PHY which is not compatible
with the older R-Car H1 (R8A7779) version.
This adds OF/platform device id tables and PHY initialization
callbacks for the following Gen2 SoCs:
  * R-Car H2: R8A7790;
  * R-Car M2: R8A7791.

PHY initialization method is chosen, based on the device id.
Default PHY settings are applied for Gen2 SoCs, which should
suit the Gen2 boards available.

The R8A7779 platform code is modified to use "sata-r8a7779"
device id.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
 arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
 arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
 drivers/ata/sata_rcar.c                            | 112 ++++++++++++++++++---
 4 files changed, 102 insertions(+), 19 deletions(-)

Comments

Mark Rutland Oct. 14, 2013, 4:26 p.m. UTC | #1
On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
> R-Car Gen2 SoCs have a different PHY which is not compatible
> with the older R-Car H1 (R8A7779) version.
> This adds OF/platform device id tables and PHY initialization
> callbacks for the following Gen2 SoCs:
>   * R-Car H2: R8A7790;
>   * R-Car M2: R8A7791.
> 
> PHY initialization method is chosen, based on the device id.
> Default PHY settings are applied for Gen2 SoCs, which should
> suit the Gen2 boards available.
> 
> The R8A7779 platform code is modified to use "sata-r8a7779"
> device id.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
>  arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
>  arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
>  drivers/ata/sata_rcar.c                            | 112 ++++++++++++++++++---
>  4 files changed, 102 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> index 2465183..b5a41bf 100644
> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> @@ -1,7 +1,10 @@
>  * Renesas R-Car SATA
>  
>  Required properties:
> -- compatible		: must be "renesas,sata-r8a7779"
> +- compatible		: must be one of the following:

s/must be/should contain/

> +			  - "renesas,sata-r8a7779" for R-Car H1
> +			  - "renesas,sata-r8a7790" for R-Car H2
> +			  - "renesas,sata-r8a7791" for R-Car M2

How do renesas,sata-r8a7790 and renesas,sata-r8a7791 differ?

>  - reg			: address range of the SATA registers.

It's a size too...

>  - interrupt-parent	: interrupt parent controller phandle

Not required.

[...]

> +static struct of_device_id sata_rcar_match[] = {
> +	{
> +		.compatible = "renesas,sata-r8a7779",
> +		.data = (void *)RCAR_GEN1_SATA,
> +	},
> +	{
> +		.compatible = "renesas,sata-r8a7790",
> +		.data = (void *)RCAR_GEN2_SATA
> +	},
> +	{
> +		.compatible = "renesas,sata-r8a7791",
> +		.data = (void *)RCAR_GEN2_SATA
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sata_rcar_match);

Are the renesas,sata-r8a779x variants identical?

If so, why the two strings?

Could we not require sata-r8a7791 devices to have "sata-r8a7790" in the
compatible list:

	compatible = "sata-r8a7791", "sata-r8a7790";

That way we can match on "sata-r8a7791" if we want to later, but don't
need code for it now.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Valentine Barshak Oct. 14, 2013, 5:58 p.m. UTC | #2
On 10/14/2013 08:26 PM, Mark Rutland wrote:
> On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
>> R-Car Gen2 SoCs have a different PHY which is not compatible
>> with the older R-Car H1 (R8A7779) version.
>> This adds OF/platform device id tables and PHY initialization
>> callbacks for the following Gen2 SoCs:
>>    * R-Car H2: R8A7790;
>>    * R-Car M2: R8A7791.
>>
>> PHY initialization method is chosen, based on the device id.
>> Default PHY settings are applied for Gen2 SoCs, which should
>> suit the Gen2 boards available.
>>
>> The R8A7779 platform code is modified to use "sata-r8a7779"
>> device id.
>>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
>>   arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
>>   arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
>>   drivers/ata/sata_rcar.c                            | 112 ++++++++++++++++++---
>>   4 files changed, 102 insertions(+), 19 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
>> index 2465183..b5a41bf 100644
>> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
>> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
>> @@ -1,7 +1,10 @@
>>   * Renesas R-Car SATA
>>
>>   Required properties:
>> -- compatible		: must be "renesas,sata-r8a7779"
>> +- compatible		: must be one of the following:
>
> s/must be/should contain/
>
>> +			  - "renesas,sata-r8a7779" for R-Car H1
>> +			  - "renesas,sata-r8a7790" for R-Car H2
>> +			  - "renesas,sata-r8a7791" for R-Car M2
>
> How do renesas,sata-r8a7790 and renesas,sata-r8a7791 differ?
>
>>   - reg			: address range of the SATA registers.
>
> It's a size too...
>
>>   - interrupt-parent	: interrupt parent controller phandle
>
> Not required.
>
> [...]
>
>> +static struct of_device_id sata_rcar_match[] = {
>> +	{
>> +		.compatible = "renesas,sata-r8a7779",
>> +		.data = (void *)RCAR_GEN1_SATA,
>> +	},
>> +	{
>> +		.compatible = "renesas,sata-r8a7790",
>> +		.data = (void *)RCAR_GEN2_SATA
>> +	},
>> +	{
>> +		.compatible = "renesas,sata-r8a7791",
>> +		.data = (void *)RCAR_GEN2_SATA
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, sata_rcar_match);
>
> Are the renesas,sata-r8a779x variants identical?
>

Yes.

> If so, why the two strings?

Just thought the driver should support "renesas,sata-r8a7791" 
compatibility as well since it should support both SoCs.
Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.

>
> Could we not require sata-r8a7791 devices to have "sata-r8a7790" in the
> compatible list:
>
> 	compatible = "sata-r8a7791", "sata-r8a7790";
>
> That way we can match on "sata-r8a7791" if we want to later, but don't
> need code for it now.

We could. In this case having compatible = "renesas,sata-r8a7791" would 
not be enough, though it looks like a valid compatible list to me.

>
> Thanks,
> Mark.
>

Thanks,
Val.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Oct. 14, 2013, 6:15 p.m. UTC | #3
Hi Valentine,

On Monday 14 October 2013 21:58:50 Valentine wrote:
> On 10/14/2013 08:26 PM, Mark Rutland wrote:
> > On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
> >> R-Car Gen2 SoCs have a different PHY which is not compatible
> >> with the older R-Car H1 (R8A7779) version.
> >> This adds OF/platform device id tables and PHY initialization
> >> 
> >> callbacks for the following Gen2 SoCs:
> >>    * R-Car H2: R8A7790;
> >>    * R-Car M2: R8A7791.
> >> 
> >> PHY initialization method is chosen, based on the device id.
> >> Default PHY settings are applied for Gen2 SoCs, which should
> >> suit the Gen2 boards available.
> >> 
> >> The R8A7779 platform code is modified to use "sata-r8a7779"
> >> device id.
> >> 
> >> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >> ---
> >> 
> >>   .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
> >>   arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
> >>   arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
> >>   drivers/ata/sata_rcar.c                            | 112 +++++++++++---
> >>   4 files changed, 102 insertions(+), 19 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >> b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> >> 2465183..b5a41bf 100644
> >> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >> @@ -1,7 +1,10 @@
> >> 
> >>   * Renesas R-Car SATA
> >> 
> >>   Required properties:
> >> -- compatible		: must be "renesas,sata-r8a7779"
> > 
> >> +- compatible		: must be one of the following:
> > s/must be/should contain/
> > 
> >> +			  - "renesas,sata-r8a7779" for R-Car H1
> >> +			  - "renesas,sata-r8a7790" for R-Car H2
> >> +			  - "renesas,sata-r8a7791" for R-Car M2
> > 
> > How do renesas,sata-r8a7790 and renesas,sata-r8a7791 differ?
> > 
> >>   - reg			: address range of the SATA registers.
> > 
> > It's a size too...
> > 
> >>   - interrupt-parent	: interrupt parent controller phandle
> > 
> > Not required.
> > 
> > [...]
> > 
> >> +static struct of_device_id sata_rcar_match[] = {
> >> +	{
> >> +		.compatible = "renesas,sata-r8a7779",
> >> +		.data = (void *)RCAR_GEN1_SATA,
> >> +	},
> >> +	{
> >> +		.compatible = "renesas,sata-r8a7790",
> >> +		.data = (void *)RCAR_GEN2_SATA
> >> +	},
> >> +	{
> >> +		.compatible = "renesas,sata-r8a7791",
> >> +		.data = (void *)RCAR_GEN2_SATA
> >> +	},
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, sata_rcar_match);
> > 
> > Are the renesas,sata-r8a779x variants identical?
> 
> Yes.
> 
> > If so, why the two strings?
> 
> Just thought the driver should support "renesas,sata-r8a7791"
> compatibility as well since it should support both SoCs.
> Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.

One reason for two compatibility strings is not to be stuck if we later find 
out the the 7791 SATA controller differs from the 7790.

> > Could we not require sata-r8a7791 devices to have "sata-r8a7790" in the
> > 
> > compatible list:
> > 	compatible = "sata-r8a7791", "sata-r8a7790";
> > 
> > That way we can match on "sata-r8a7791" if we want to later, but don't
> > need code for it now.
> 
> We could. In this case having compatible = "renesas,sata-r8a7791" would not
> be enough, though it looks like a valid compatible list to me.

I'm fine with both solution, as long as the "sata-r8a7791" compatibility 
string is present and has precedence over "sata-r8a7790".
Valentine Barshak Oct. 14, 2013, 9:22 p.m. UTC | #4
On 10/14/2013 10:15 PM, Laurent Pinchart wrote:
> Hi Valentine,
>

Hi Laurent,

> On Monday 14 October 2013 21:58:50 Valentine wrote:
>> On 10/14/2013 08:26 PM, Mark Rutland wrote:
>>> On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
>>>> R-Car Gen2 SoCs have a different PHY which is not compatible
>>>> with the older R-Car H1 (R8A7779) version.
>>>> This adds OF/platform device id tables and PHY initialization
>>>>
>>>> callbacks for the following Gen2 SoCs:
>>>>     * R-Car H2: R8A7790;
>>>>     * R-Car M2: R8A7791.
>>>>
>>>> PHY initialization method is chosen, based on the device id.
>>>> Default PHY settings are applied for Gen2 SoCs, which should
>>>> suit the Gen2 boards available.
>>>>
>>>> The R8A7779 platform code is modified to use "sata-r8a7779"
>>>> device id.
>>>>
>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>>> ---
>>>>
>>>>    .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
>>>>    arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
>>>>    arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
>>>>    drivers/ata/sata_rcar.c                            | 112 +++++++++++---
>>>>    4 files changed, 102 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
>>>> b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
>>>> 2465183..b5a41bf 100644
>>>> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
>>>> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
>>>> @@ -1,7 +1,10 @@
>>>>
>>>>    * Renesas R-Car SATA
>>>>
>>>>    Required properties:
>>>> -- compatible		: must be "renesas,sata-r8a7779"
>>>
>>>> +- compatible		: must be one of the following:
>>> s/must be/should contain/
>>>
>>>> +			  - "renesas,sata-r8a7779" for R-Car H1
>>>> +			  - "renesas,sata-r8a7790" for R-Car H2
>>>> +			  - "renesas,sata-r8a7791" for R-Car M2
>>>
>>> How do renesas,sata-r8a7790 and renesas,sata-r8a7791 differ?
>>>
>>>>    - reg			: address range of the SATA registers.
>>>
>>> It's a size too...
>>>
>>>>    - interrupt-parent	: interrupt parent controller phandle
>>>
>>> Not required.
>>>
>>> [...]
>>>
>>>> +static struct of_device_id sata_rcar_match[] = {
>>>> +	{
>>>> +		.compatible = "renesas,sata-r8a7779",
>>>> +		.data = (void *)RCAR_GEN1_SATA,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "renesas,sata-r8a7790",
>>>> +		.data = (void *)RCAR_GEN2_SATA
>>>> +	},
>>>> +	{
>>>> +		.compatible = "renesas,sata-r8a7791",
>>>> +		.data = (void *)RCAR_GEN2_SATA
>>>> +	},
>>>> +	{},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, sata_rcar_match);
>>>
>>> Are the renesas,sata-r8a779x variants identical?
>>
>> Yes.
>>
>>> If so, why the two strings?
>>
>> Just thought the driver should support "renesas,sata-r8a7791"
>> compatibility as well since it should support both SoCs.
>> Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.
>
> One reason for two compatibility strings is not to be stuck if we later find
> out the the 7791 SATA controller differs from the 7790.
>

Thanks!
Yeah, anyways, I'd prefer to keep both strings in the device id table.

>>> Could we not require sata-r8a7791 devices to have "sata-r8a7790" in the
>>>
>>> compatible list:
>>> 	compatible = "sata-r8a7791", "sata-r8a7790";
>>>
>>> That way we can match on "sata-r8a7791" if we want to later, but don't
>>> need code for it now.
>>
>> We could. In this case having compatible = "renesas,sata-r8a7791" would not
>> be enough, though it looks like a valid compatible list to me.
>
> I'm fine with both solution, as long as the "sata-r8a7791" compatibility
> string is present and has precedence over "sata-r8a7790".
>

Thanks,
Val.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Oct. 15, 2013, 12:23 a.m. UTC | #5
On Tue, Oct 15, 2013 at 01:22:23AM +0400, Valentine wrote:
> On 10/14/2013 10:15 PM, Laurent Pinchart wrote:
> >Hi Valentine,
> >
> 
> Hi Laurent,
> 
> >On Monday 14 October 2013 21:58:50 Valentine wrote:
> >>On 10/14/2013 08:26 PM, Mark Rutland wrote:
> >>>On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
> >>>>R-Car Gen2 SoCs have a different PHY which is not compatible
> >>>>with the older R-Car H1 (R8A7779) version.
> >>>>This adds OF/platform device id tables and PHY initialization
> >>>>
> >>>>callbacks for the following Gen2 SoCs:
> >>>>    * R-Car H2: R8A7790;
> >>>>    * R-Car M2: R8A7791.
> >>>>
> >>>>PHY initialization method is chosen, based on the device id.
> >>>>Default PHY settings are applied for Gen2 SoCs, which should
> >>>>suit the Gen2 boards available.
> >>>>
> >>>>The R8A7779 platform code is modified to use "sata-r8a7779"
> >>>>device id.
> >>>>
> >>>>Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >>>>---
> >>>>
> >>>>   .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
> >>>>   arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
> >>>>   arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
> >>>>   drivers/ata/sata_rcar.c                            | 112 +++++++++++---
> >>>>   4 files changed, 102 insertions(+), 19 deletions(-)
> >>>>
> >>>>diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >>>>b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> >>>>2465183..b5a41bf 100644
> >>>>--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >>>>+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> >>>>@@ -1,7 +1,10 @@
> >>>>
> >>>>   * Renesas R-Car SATA
> >>>>
> >>>>   Required properties:
> >>>>-- compatible		: must be "renesas,sata-r8a7779"
> >>>
> >>>>+- compatible		: must be one of the following:
> >>>s/must be/should contain/
> >>>
> >>>>+			  - "renesas,sata-r8a7779" for R-Car H1
> >>>>+			  - "renesas,sata-r8a7790" for R-Car H2
> >>>>+			  - "renesas,sata-r8a7791" for R-Car M2
> >>>
> >>>How do renesas,sata-r8a7790 and renesas,sata-r8a7791 differ?
> >>>
> >>>>   - reg			: address range of the SATA registers.
> >>>
> >>>It's a size too...
> >>>
> >>>>   - interrupt-parent	: interrupt parent controller phandle
> >>>
> >>>Not required.
> >>>
> >>>[...]
> >>>
> >>>>+static struct of_device_id sata_rcar_match[] = {
> >>>>+	{
> >>>>+		.compatible = "renesas,sata-r8a7779",
> >>>>+		.data = (void *)RCAR_GEN1_SATA,
> >>>>+	},
> >>>>+	{
> >>>>+		.compatible = "renesas,sata-r8a7790",
> >>>>+		.data = (void *)RCAR_GEN2_SATA
> >>>>+	},
> >>>>+	{
> >>>>+		.compatible = "renesas,sata-r8a7791",
> >>>>+		.data = (void *)RCAR_GEN2_SATA
> >>>>+	},
> >>>>+	{},
> >>>>+};
> >>>>+MODULE_DEVICE_TABLE(of, sata_rcar_match);
> >>>
> >>>Are the renesas,sata-r8a779x variants identical?
> >>
> >>Yes.
> >>
> >>>If so, why the two strings?
> >>
> >>Just thought the driver should support "renesas,sata-r8a7791"
> >>compatibility as well since it should support both SoCs.
> >>Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.
> >
> >One reason for two compatibility strings is not to be stuck if we later find
> >out the the 7791 SATA controller differs from the 7790.
> >
> 
> Thanks!
> Yeah, anyways, I'd prefer to keep both strings in the device id table.

Yes, I do too, mostly for the reason that Laurent gave.
But also because this is consistent with compatibility strings
for other Renesas IP.

> >>>Could we not require sata-r8a7791 devices to have "sata-r8a7790" in the
> >>>
> >>>compatible list:
> >>>	compatible = "sata-r8a7791", "sata-r8a7790";
> >>>
> >>>That way we can match on "sata-r8a7791" if we want to later, but don't
> >>>need code for it now.
> >>
> >>We could. In this case having compatible = "renesas,sata-r8a7791" would not
> >>be enough, though it looks like a valid compatible list to me.
> >
> >I'm fine with both solution, as long as the "sata-r8a7791" compatibility
> >string is present and has precedence over "sata-r8a7790".
> >
> 
> Thanks,
> Val.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Oct. 15, 2013, 12:32 a.m. UTC | #6
Hi Simon,

On Tuesday 15 October 2013 09:23:19 Simon Horman wrote:
> On Tue, Oct 15, 2013 at 01:22:23AM +0400, Valentine wrote:
> > On 10/14/2013 10:15 PM, Laurent Pinchart wrote:
> > >On Monday 14 October 2013 21:58:50 Valentine wrote:
> > >>On 10/14/2013 08:26 PM, Mark Rutland wrote:
> > >>>On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
> > >>>>R-Car Gen2 SoCs have a different PHY which is not compatible
> > >>>>with the older R-Car H1 (R8A7779) version.
> > >>>>This adds OF/platform device id tables and PHY initialization
> > >>>>
> > >>>>callbacks for the following Gen2 SoCs:
> > >>>>    * R-Car H2: R8A7790;
> > >>>>    * R-Car M2: R8A7791.
> > >>>>
> > >>>>PHY initialization method is chosen, based on the device id.
> > >>>>Default PHY settings are applied for Gen2 SoCs, which should
> > >>>>suit the Gen2 boards available.
> > >>>>
> > >>>>The R8A7779 platform code is modified to use "sata-r8a7779"
> > >>>>device id.
> > >>>>
> > >>>>Signed-off-by: Valentine Barshak
> > >>>><valentine.barshak@cogentembedded.com>
> > >>>>---
> > >>>>
> > >>>>   .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
> > >>>>   arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
> > >>>>   arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
> > >>>>   drivers/ata/sata_rcar.c                            | 112
> > >>>>   +++++++++++---
> > >>>>   4 files changed, 102 insertions(+), 19 deletions(-)
> > >>>>
> > >>>>diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > >>>>b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> > >>>>2465183..b5a41bf 100644
> > >>>>--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > >>>>+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt

[snip]

> > >>>>+static struct of_device_id sata_rcar_match[] = {
> > >>>>+	{
> > >>>>+		.compatible = "renesas,sata-r8a7779",
> > >>>>+		.data = (void *)RCAR_GEN1_SATA,
> > >>>>+	},
> > >>>>+	{
> > >>>>+		.compatible = "renesas,sata-r8a7790",
> > >>>>+		.data = (void *)RCAR_GEN2_SATA
> > >>>>+	},
> > >>>>+	{
> > >>>>+		.compatible = "renesas,sata-r8a7791",
> > >>>>+		.data = (void *)RCAR_GEN2_SATA
> > >>>>+	},
> > >>>>+	{},
> > >>>>+};
> > >>>>+MODULE_DEVICE_TABLE(of, sata_rcar_match);
> > >>>
> > >>>Are the renesas,sata-r8a779x variants identical?
> > >>
> > >>Yes.
> > >>
> > >>>If so, why the two strings?
> > >>
> > >>Just thought the driver should support "renesas,sata-r8a7791"
> > >>compatibility as well since it should support both SoCs.
> > >>Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.
> > >
> > >One reason for two compatibility strings is not to be stuck if we later
> > >find out the the 7791 SATA controller differs from the 7790.
> > 
> > Thanks!
> > Yeah, anyways, I'd prefer to keep both strings in the device id table.
> 
> Yes, I do too, mostly for the reason that Laurent gave.
> But also because this is consistent with compatibility strings
> for other Renesas IP.

We need to use the 7791 compatibility string in DT, but if we specify both, 
the driver could have a single entry for both SoCs in its device ID table. As 
stated before, I have no strong preference, both would work.
Simon Horman Oct. 16, 2013, 6:10 a.m. UTC | #7
On Tue, Oct 15, 2013 at 02:32:28AM +0200, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Tuesday 15 October 2013 09:23:19 Simon Horman wrote:
> > On Tue, Oct 15, 2013 at 01:22:23AM +0400, Valentine wrote:
> > > On 10/14/2013 10:15 PM, Laurent Pinchart wrote:
> > > >On Monday 14 October 2013 21:58:50 Valentine wrote:
> > > >>On 10/14/2013 08:26 PM, Mark Rutland wrote:
> > > >>>On Mon, Oct 14, 2013 at 04:42:34PM +0100, Valentine Barshak wrote:
> > > >>>>R-Car Gen2 SoCs have a different PHY which is not compatible
> > > >>>>with the older R-Car H1 (R8A7779) version.
> > > >>>>This adds OF/platform device id tables and PHY initialization
> > > >>>>
> > > >>>>callbacks for the following Gen2 SoCs:
> > > >>>>    * R-Car H2: R8A7790;
> > > >>>>    * R-Car M2: R8A7791.
> > > >>>>
> > > >>>>PHY initialization method is chosen, based on the device id.
> > > >>>>Default PHY settings are applied for Gen2 SoCs, which should
> > > >>>>suit the Gen2 boards available.
> > > >>>>
> > > >>>>The R8A7779 platform code is modified to use "sata-r8a7779"
> > > >>>>device id.
> > > >>>>
> > > >>>>Signed-off-by: Valentine Barshak
> > > >>>><valentine.barshak@cogentembedded.com>
> > > >>>>---
> > > >>>>
> > > >>>>   .../devicetree/bindings/ata/sata_rcar.txt          |   5 +-
> > > >>>>   arch/arm/mach-shmobile/clock-r8a7779.c             |   2 +-
> > > >>>>   arch/arm/mach-shmobile/setup-r8a7779.c             |   2 +-
> > > >>>>   drivers/ata/sata_rcar.c                            | 112
> > > >>>>   +++++++++++---
> > > >>>>   4 files changed, 102 insertions(+), 19 deletions(-)
> > > >>>>
> > > >>>>diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > > >>>>b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> > > >>>>2465183..b5a41bf 100644
> > > >>>>--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > > >>>>+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> 
> [snip]
> 
> > > >>>>+static struct of_device_id sata_rcar_match[] = {
> > > >>>>+	{
> > > >>>>+		.compatible = "renesas,sata-r8a7779",
> > > >>>>+		.data = (void *)RCAR_GEN1_SATA,
> > > >>>>+	},
> > > >>>>+	{
> > > >>>>+		.compatible = "renesas,sata-r8a7790",
> > > >>>>+		.data = (void *)RCAR_GEN2_SATA
> > > >>>>+	},
> > > >>>>+	{
> > > >>>>+		.compatible = "renesas,sata-r8a7791",
> > > >>>>+		.data = (void *)RCAR_GEN2_SATA
> > > >>>>+	},
> > > >>>>+	{},
> > > >>>>+};
> > > >>>>+MODULE_DEVICE_TABLE(of, sata_rcar_match);
> > > >>>
> > > >>>Are the renesas,sata-r8a779x variants identical?
> > > >>
> > > >>Yes.
> > > >>
> > > >>>If so, why the two strings?
> > > >>
> > > >>Just thought the driver should support "renesas,sata-r8a7791"
> > > >>compatibility as well since it should support both SoCs.
> > > >>Didn't want to force sata-r8a7790 compatibility for the r8a7791 SoC DTS.
> > > >
> > > >One reason for two compatibility strings is not to be stuck if we later
> > > >find out the the 7791 SATA controller differs from the 7790.
> > > 
> > > Thanks!
> > > Yeah, anyways, I'd prefer to keep both strings in the device id table.
> > 
> > Yes, I do too, mostly for the reason that Laurent gave.
> > But also because this is consistent with compatibility strings
> > for other Renesas IP.
> 
> We need to use the 7791 compatibility string in DT, but if we specify both, 
> the driver could have a single entry for both SoCs in its device ID table. As 
> stated before, I have no strong preference, both would work.

Thanks, I understand.

At this stage I don't have a strong preference either.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 2465183..b5a41bf 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -1,7 +1,10 @@ 
 * Renesas R-Car SATA
 
 Required properties:
-- compatible		: must be "renesas,sata-r8a7779"
+- compatible		: must be one of the following:
+			  - "renesas,sata-r8a7779" for R-Car H1
+			  - "renesas,sata-r8a7790" for R-Car H2
+			  - "renesas,sata-r8a7791" for R-Car M2
 - reg			: address range of the SATA registers.
 - interrupt-parent	: interrupt parent controller phandle
 - interrupts		: must consist of one interrupt specifier.
diff --git a/arch/arm/mach-shmobile/clock-r8a7779.c b/arch/arm/mach-shmobile/clock-r8a7779.c
index badb8b7..6593237 100644
--- a/arch/arm/mach-shmobile/clock-r8a7779.c
+++ b/arch/arm/mach-shmobile/clock-r8a7779.c
@@ -170,7 +170,7 @@  static struct clk_lookup lookups[] = {
 	/* MSTP32 clocks */
 	CLKDEV_DEV_ID("r8a7779-vin.3", &mstp_clks[MSTP120]), /* VIN3 */
 	CLKDEV_DEV_ID("rcar-pcie", &mstp_clks[MSTP116]), /* PCIe */
-	CLKDEV_DEV_ID("sata_rcar", &mstp_clks[MSTP115]), /* SATA */
+	CLKDEV_DEV_ID("sata-r8a7779", &mstp_clks[MSTP115]), /* SATA */
 	CLKDEV_DEV_ID("fc600000.sata", &mstp_clks[MSTP115]), /* SATA w/DT */
 	CLKDEV_DEV_ID("r8a777x-ether", &mstp_clks[MSTP114]), /* Ether */
 	CLKDEV_DEV_ID("r8a7779-vin.0", &mstp_clks[MSTP110]), /* VIN0 */
diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
index 13049e9..6aac99e 100644
--- a/arch/arm/mach-shmobile/setup-r8a7779.c
+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
@@ -438,7 +438,7 @@  static struct resource sata_resources[] = {
 };
 
 static struct platform_device sata_device = {
-	.name		= "sata_rcar",
+	.name		= "sata-r8a7779",
 	.id		= -1,
 	.resource	= sata_resources,
 	.num_resources	= ARRAY_SIZE(sata_resources),
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index c4cd738..80fc2e1 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -15,6 +15,7 @@ 
 #include <linux/module.h>
 #include <linux/ata.h>
 #include <linux/libata.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/err.h>
@@ -123,12 +124,37 @@ 
 
 #define SATA_RCAR_DMA_BOUNDARY		0x1FFFFFFEUL
 
+/* Gen2 Physical Layer Control Registers */
+#define RCAR_GEN2_PHY_CTL1_REG		0x1704
+#define RCAR_GEN2_PHY_CTL1		0x34180002
+#define RCAR_GEN2_PHY_CTL1_SS		0xC180	/* Spread Spectrum */
+
+#define RCAR_GEN2_PHY_CTL2_REG		0x170C
+#define RCAR_GEN2_PHY_CTL2		0x00002303
+
+#define RCAR_GEN2_PHY_CTL3_REG		0x171C
+#define RCAR_GEN2_PHY_CTL3		0x000B0194
+
+#define RCAR_GEN2_PHY_CTL4_REG		0x1724
+#define RCAR_GEN2_PHY_CTL4		0x00030994
+
+#define RCAR_GEN2_PHY_CTL5_REG		0x1740
+#define RCAR_GEN2_PHY_CTL5		0x03004001
+#define RCAR_GEN2_PHY_CTL5_DC		BIT(1)	/* DC connection */
+#define RCAR_GEN2_PHY_CTL5_TR		BIT(2)	/* Termination Resistor */
+
+enum sata_rcar_type {
+	RCAR_GEN1_SATA,
+	RCAR_GEN2_SATA,
+};
+
 struct sata_rcar_priv {
 	void __iomem *base;
 	struct clk *clk;
+	enum sata_rcar_type type;
 };
 
-static void sata_rcar_phy_initialize(struct sata_rcar_priv *priv)
+static void sata_rcar_gen1_phy_preinit(struct sata_rcar_priv *priv)
 {
 	void __iomem *base = priv->base;
 
@@ -141,8 +167,8 @@  static void sata_rcar_phy_initialize(struct sata_rcar_priv *priv)
 	iowrite32(0, base + SATAPHYRESET_REG);
 }
 
-static void sata_rcar_phy_write(struct sata_rcar_priv *priv, u16 reg, u32 val,
-				int group)
+static void sata_rcar_gen1_phy_write(struct sata_rcar_priv *priv, u16 reg,
+				     u32 val, int group)
 {
 	void __iomem *base = priv->base;
 	int timeout;
@@ -170,6 +196,29 @@  static void sata_rcar_phy_write(struct sata_rcar_priv *priv, u16 reg, u32 val,
 	iowrite32(0, base + SATAPHYADDR_REG);
 }
 
+static void sata_rcar_gen1_phy_init(struct sata_rcar_priv *priv)
+{
+	sata_rcar_gen1_phy_preinit(priv);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1);
+	sata_rcar_gen1_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0);
+}
+
+static void sata_rcar_gen2_phy_init(struct sata_rcar_priv *priv)
+{
+	void __iomem *base = priv->base;
+
+	iowrite32(RCAR_GEN2_PHY_CTL1, base + RCAR_GEN2_PHY_CTL1_REG);
+	iowrite32(RCAR_GEN2_PHY_CTL2, base + RCAR_GEN2_PHY_CTL2_REG);
+	iowrite32(RCAR_GEN2_PHY_CTL3, base + RCAR_GEN2_PHY_CTL3_REG);
+	iowrite32(RCAR_GEN2_PHY_CTL4, base + RCAR_GEN2_PHY_CTL4_REG);
+	iowrite32(RCAR_GEN2_PHY_CTL5 | RCAR_GEN2_PHY_CTL5_DC |
+		  RCAR_GEN2_PHY_CTL5_TR, base + RCAR_GEN2_PHY_CTL5_REG);
+}
+
 static void sata_rcar_freeze(struct ata_port *ap)
 {
 	struct sata_rcar_priv *priv = ap->host->private_data;
@@ -738,13 +787,17 @@  static void sata_rcar_init_controller(struct ata_host *host)
 	u32 val;
 
 	/* reset and setup phy */
-	sata_rcar_phy_initialize(priv);
-	sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 0);
-	sata_rcar_phy_write(priv, SATAPCTLR1_REG, 0x00200188, 1);
-	sata_rcar_phy_write(priv, SATAPCTLR3_REG, 0x0000A061, 0);
-	sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 0);
-	sata_rcar_phy_write(priv, SATAPCTLR2_REG, 0x20000000, 1);
-	sata_rcar_phy_write(priv, SATAPCTLR4_REG, 0x28E80000, 0);
+	switch (priv->type) {
+	case RCAR_GEN1_SATA:
+		sata_rcar_gen1_phy_init(priv);
+		break;
+	case RCAR_GEN2_SATA:
+		sata_rcar_gen2_phy_init(priv);
+		break;
+	default:
+		dev_warn(host->dev, "SATA phy is not initialized\n");
+		break;
+	}
 
 	/* SATA-IP reset state */
 	val = ioread32(base + ATAPI_CONTROL1_REG);
@@ -770,8 +823,34 @@  static void sata_rcar_init_controller(struct ata_host *host)
 	iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG);
 }
 
+static struct of_device_id sata_rcar_match[] = {
+	{
+		.compatible = "renesas,sata-r8a7779",
+		.data = (void *)RCAR_GEN1_SATA,
+	},
+	{
+		.compatible = "renesas,sata-r8a7790",
+		.data = (void *)RCAR_GEN2_SATA
+	},
+	{
+		.compatible = "renesas,sata-r8a7791",
+		.data = (void *)RCAR_GEN2_SATA
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sata_rcar_match);
+
+static const struct platform_device_id sata_rcar_id_table[] = {
+	{ "sata-r8a7779", RCAR_GEN1_SATA },
+	{ "sata-r8a7790", RCAR_GEN2_SATA },
+	{ "sata-r8a7791", RCAR_GEN2_SATA },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, sata_rcar_id_table);
+
 static int sata_rcar_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
 	struct ata_host *host;
 	struct sata_rcar_priv *priv;
 	struct resource *mem;
@@ -787,6 +866,12 @@  static int sata_rcar_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	of_id = of_match_device(sata_rcar_match, &pdev->dev);
+	if (of_id)
+		priv->type = (enum sata_rcar_type)of_id->data;
+	else
+		priv->type = platform_get_device_id(pdev)->driver_data;
+
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		dev_err(&pdev->dev, "failed to get access to sata clock\n");
@@ -892,15 +977,10 @@  static const struct dev_pm_ops sata_rcar_pm_ops = {
 };
 #endif
 
-static struct of_device_id sata_rcar_match[] = {
-	{ .compatible = "renesas,sata-r8a7779", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, sata_rcar_match);
-
 static struct platform_driver sata_rcar_driver = {
 	.probe		= sata_rcar_probe,
 	.remove		= sata_rcar_remove,
+	.id_table	= sata_rcar_id_table,
 	.driver = {
 		.name		= DRV_NAME,
 		.owner		= THIS_MODULE,