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. Thanks Kishon > the already existing 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