On Tue, Nov 19, 2013 at 3:22 PM, Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
> On Friday 15 November 2013 11:17 AM, Yuvaraj Kumar wrote:
>> On Thu, Nov 14, 2013 at 11:18 AM, Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
>>> Hi,
>>>
>>> On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
>>>> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
>>>>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>>>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>>>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>>>>> So this patch also adds a i2c client driver, which is used configure
>>>>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>>>>
>>>>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>>>>
>>>>>> This patch incorporates the generic phy framework to deal with sata
>>>>>> phy.
>>>>>>
>>>>>> This patch depends on the below patch
>>>>>> [1].drivers: phy: add generic PHY framework
>>>>>> by Kishon Vijay Abraham I<kishon@xxxxxx>
>>>>>>
>>>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@xxxxxxxxxxx>
>>>>>> Signed-off-by: Girish K S <ks.giri@xxxxxxxxxxx>
>>>>>> Signed-off-by: Vasanth Ananthan <vasanth.a@xxxxxxxxxxx>
>>>>>> ---
>>>>>> drivers/phy/Kconfig | 6 +
>>>>>> drivers/phy/Makefile | 1 +
>>>>>> drivers/phy/exynos/Kconfig | 5 +
>>>>>> drivers/phy/exynos/Makefile | 5 +
>>>>>> drivers/phy/exynos/exynos5250_phy_i2c.c | 53 +++++++
>>>>>> drivers/phy/exynos/sata_phy_exynos5250.c | 248 ++++++++++++++++++++++++++++++
>>>>>> drivers/phy/exynos/sata_phy_exynos5250.h | 33 ++++
>>>>>> 7 files changed, 351 insertions(+)
>>>>>> create mode 100644 drivers/phy/exynos/Kconfig
>>>>>> create mode 100644 drivers/phy/exynos/Makefile
>>>>>> create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>> create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>>>> create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>>>>
>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>> index 5f85909..ab3d1c6 100644
>>>>>> --- a/drivers/phy/Kconfig
>>>>>> +++ b/drivers/phy/Kconfig
>>>>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>>>> devices present in the kernel. This layer will have the generic
>>>>>> API by which phy drivers can create PHY using the phy framework and
>>>>>> phy users can obtain reference to the PHY.
>>>>>> +
>>>>>> +if GENERIC_PHY
>>>>>
>>>>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>>>>> +
>>>>>> +source "drivers/phy/exynos/Kconfig"
>>>>>> +
>>>>>> +endif
>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>> index 9e9560f..e0223d7 100644
>>>>>> --- a/drivers/phy/Makefile
>>>>>> +++ b/drivers/phy/Makefile
>>>>>> @@ -3,3 +3,4 @@
>>>>>> #
>>>>>>
>>>>>> obj-$(CONFIG_GENERIC_PHY) += phy-core.o
>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA) += exynos/
>>>>>
>>>>> simply have phy-exynos5250 in drivers/phy.
>>>> ok.
>>>>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>>>>> new file mode 100644
>>>>>> index 0000000..fa125fb
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/Kconfig
>>>>>> @@ -0,0 +1,5 @@
>>>>>> +config PHY_SAMSUNG_SATA
>>>>>> + tristate "Samsung Sata SerDes/PHY driver"
>>>>>> + help
>>>>>> + Support for Samsung sata SerDes/Phy found on Samsung
>>>>>> + SoCs.
>>>>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>>>>> new file mode 100644
>>>>>> index 0000000..50dc7eb
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/Makefile
>>>>>> @@ -0,0 +1,5 @@
>>>>>> +#
>>>>>> +# Makefile for the exynos phy drivers.
>>>>>> +#
>>>>>> +ccflags-y := -Idrivers/phy/exynos
>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA) += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>>>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>> new file mode 100644
>>>>>> index 0000000..9c75d3b
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>> @@ -0,0 +1,53 @@
>>>>>> +/*
>>>>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>>>>> + * Author:
>>>>>> + * Yuvaraj C D <yuvaraj.cd@xxxxxxxxxxx>
>>>>>> + *
>>>>>> + * This program is free software; you can redistribute it and/or modify it
>>>>>> + * under the terms of the GNU General Public License as published by the
>>>>>> + * Free Software Foundation; either version 2 of the License, or (at your
>>>>>> + * option) any later version.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/kernel.h>
>>>>>> +#include <linux/i2c.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include "sata_phy_exynos5250.h"
>>>>>> +
>>>>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>>>>> + const struct i2c_device_id *i2c_id)
>>>>>> +{
>>>>>> + sataphy_attach_i2c_client(client);
>>>>>> +
>>>>>> + dev_info(&client->adapter->dev,
>>>>>> + "attached %s into i2c adapter successfully\n",
>>>>>> + client->name);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>>>>> +{
>>>>>> + dev_info(&client->adapter->dev,
>>>>>> + "detached %s from i2c adapter successfully\n",
>>>>>> + client->name);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>>>>> + { "sata-phy-i2c", 0 },
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>>>>> +
>>>>>> +struct i2c_driver sataphy_i2c_driver = {
>>>>>> + .probe = exynos_sata_i2c_probe,
>>>>>> + .id_table = phy_i2c_device_match,
>>>>>> + .remove = exynos_sata_i2c_remove,
>>>>>> + .driver = {
>>>>>> + .name = "sata-phy-i2c",
>>>>>> + .owner = THIS_MODULE,
>>>>>> + .of_match_table = (void *)phy_i2c_device_match,
>>>>>> + },
>>>>>> +};
>>>>>
>>>>> As I just mentioned above, we can merge this driver with the below one.
>>>> True, Initially it was merged.But already existing drivers of which
>>>> are of similar to this kind were done in this way.
>>>> Please refer /drivers/gpu/drm//exynos/exynos_hdmiphy.c
>>>
>>> Can you point to any discussions where it was decided to go with this approach?
>> Sorry,what I meant is exynos_hdmiphy.c is already exist in the
>> mainline with this approach.
>>
>> Initially it was merged on my local patches(Not upstreamed).But after
>> referring to the
>> exynos_hdmiphy.c driver and with lot of internal discussions ,it was
>> decided to go with
>
> I don't have any idea about the internal discussions whatever you had. IMO
> Exynos5250 sata phy driver should be modelled as an i2c client driver if you
> don't have a good reason to do otherwise.
In Exynos5250,some of the registers of SATA PHY controller are I/O
mapped and some are accessible
only through I2C controller.
As the whole registers of SATA PHY controller cannot be accessible
through I2C bus, it cannot be made as an i2c client driver.
>
> Thanks
> Kishon
>
>> the already existing approach.
>>
>>>
>>> Thanks
>>> Kishon
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
