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?
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
