RE: [PATCH 1/5 RESEND] cpufreq: exynos: Remove unused variable & IS_ERR
Rafael J. Wysocki wrote: > > On Tuesday, December 18, 2012 08:04:45 PM Kukjin Kim wrote: > > Jonghwan Choi wrote: > > > > > > The variable 'max_support_idx, min_support_idx, pm_lock_idx" > > > are never used, so remove the unused variable. > > > > > > Signed-off-by: Jonghwan Choi > > > --- > > > > Basically, looks OK to me... > > > > Rafael, are you taking this series in your tree? > > Or let me pick into Samsung tree? > > Please take it to the Samsung tree, if that's not a problem. > OK, let me pick this up. > I generally prefer driver changes to go through platform trees where they > can > get more testing coverage. > Yes, if any problems, let you know. Thanks. - Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V5 2/2] ASoC: SAMSUNG: Add DT support for i2s
Padma Venkat wrote: > > Hi, > > On Wed, Dec 19, 2012 at 10:39 PM, Mark Brown > wrote: > > On Wed, Dec 19, 2012 at 01:24:14PM +, Grant Likely wrote: > >> On Thu, 13 Dec 2012 16:12:53 +0530, Padmavathi Venna > wrote: > > > >> > +- compatible : "samsung,samsung-i2s" > > > >> Isn't that kind of redundant? :-) > > > >> The format of the compatible strings should be ", number>-i2s". > >> Please be specific about the part number that you're doing the binding > >> for. For example; use "samsung,exynos4210-i2s" instead of > "samsung,exynos-i2s". > > > > There are actually versioned IPs here (where the versions are used > > publically in a few places) but it's not clearly documented which is > > which. It would be reasonable to use the IP versions here I think. > > Samsung has three i2s drivers one for s3c24xx, one for s3c2412 and one > for rest of the platforms. The above mentioned other platforms has > Version 3/4/5 of i2s controllers. This dt binding is for for the i2s Where is the version defined such as 3, 4, 5? So, what is the "sound/soc/Samsung/s3c-i2s-v2.[ch]"? > driver that has support for Version 3/4/5 of i2s controller. So > "samsung,i2s-v5" is okay as compatible name? Please suggest me. > I agree with using version here but we need some consensus about that. - Kukjin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] ARM: dts: Fix gpio pin names of keypad columns in exynos4x12-pinctrl
Sachin Kamat wrote:
>
> Keypad columns are connected to gpx1 pins on 4x12.
>
> Cc: Tomasz Figa
> Signed-off-by: Sachin Kamat
> ---
> arch/arm/boot/dts/exynos4x12-pinctrl.dtsi | 16
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> index 8e6115a..0b293fb 100644
> --- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> @@ -779,56 +779,56 @@
> };
>
> keypad_col0: keypad-col0 {
> - samsung,pins = "gpl2-0";
> + samsung,pins = "gpx1-0";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col1: keypad-col1 {
> - samsung,pins = "gpl2-1";
> + samsung,pins = "gpx1-1";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col2: keypad-col2 {
> - samsung,pins = "gpl2-2";
> + samsung,pins = "gpx1-2";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col3: keypad-col3 {
> - samsung,pins = "gpl2-3";
> + samsung,pins = "gpx1-3";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col4: keypad-col4 {
> - samsung,pins = "gpl2-4";
> + samsung,pins = "gpx1-4";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col5: keypad-col5 {
> - samsung,pins = "gpl2-5";
> + samsung,pins = "gpx1-5";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col6: keypad-col6 {
> - samsung,pins = "gpl2-6";
> + samsung,pins = "gpx1-6";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> };
>
> keypad_col7: keypad-col7 {
> - samsung,pins = "gpl2-7";
> + samsung,pins = "gpx1-7";
> samsung,pin-function = <3>;
> samsung,pin-pud = <0>;
> samsung,pin-drv = <0>;
> --
> 1.7.4.1
As I commented in your other patch, the value should be defined at the board
dt file not here. And I think, need to fix this file.
- Kukjin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] ARM: dts: Add keypad row entries for exynos4x12
Sachin Kamat wrote:
>
> This patch adds keypad row support for Exynos4x12.
>
> Signed-off-by: Sachin Kamat
> ---
> arch/arm/boot/dts/exynos4x12-pinctrl.dtsi | 56
> +
> 1 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> index 0b293fb..bdfdf51 100644
> --- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
> @@ -778,6 +778,62 @@
> samsung,pin-drv = <3>;
> };
>
> + keypad_row0: keypad-row0 {
> + samsung,pins = "gpx2-0";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row1: keypad-row1 {
> + samsung,pins = "gpx2-1";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row2: keypad-row2 {
> + samsung,pins = "gpx2-2";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row3: keypad-row3 {
> + samsung,pins = "gpx2-3";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row4: keypad-row4 {
> + samsung,pins = "gpx2-4";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row5: keypad-row5 {
> + samsung,pins = "gpx2-5";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row6: keypad-row6 {
> + samsung,pins = "gpx2-6";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> + keypad_row7: keypad-row7 {
> + samsung,pins = "gpx2-7";
> + samsung,pin-function = <3>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +
> keypad_col0: keypad-col0 {
> samsung,pins = "gpx1-0";
> samsung,pin-function = <3>;
> --
> 1.7.4.1
No, the nodes for keypad should be defined at each own board dt file.
Because it depends on board not SoC even though some boards can be same each
other.
- Kukjin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] ARM: Exynos5250: Enabling ehci-s5p driver
Vivek,
On Fri, Dec 21, 2012 at 12:16 AM, Vivek Gautam
wrote:
> Hi all,
>
>
> On Wed, Dec 19, 2012 at 7:20 PM, Vivek Gautam
> wrote:
>> CC: Doug Anderson
>>
>>
>> On Sat, Dec 15, 2012 at 12:53 PM, Grant Likely
>> wrote:
>>> On Thu, 13 Dec 2012 22:06:01 +0530, Vivek Gautam
>>> wrote:
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c
b/arch/arm/mach-exynos/mach-exynos5-dt.c
index 462e5ac..b3b9af1 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -110,6 +110,8 @@ static const struct of_dev_auxdata
exynos5250_auxdata_lookup[] __initconst = {
"samsung-i2s.1", NULL),
OF_DEV_AUXDATA("samsung,samsung-i2s", 0x12D7,
"samsung-i2s.2", NULL),
+ OF_DEV_AUXDATA("samsung,exynos4210-ehci", EXYNOS5_PA_EHCI,
+ "s5p-ehci", NULL),
>>>
>>> I'm assuming the above change is temporary. What is left to be done to
>>> drop the auxdata in theses two patches?
>>>
>>> Otherwise the patch looks fine.
>>>
>>> Acked-by: Grant Likely
>
> Any more thought about this patch?
> Or does this change seems fine?
I'm fairly certain we can just drop the OF_DEV_AUXDATA entry here. I
haven't been following this as closely as I should, but I know that
the comment for this table says that the whole purpose is to override
device names and that it should be temporary. We don't need to do any
overriding of device names here, do we?
When I drop this (and the ohci and phy) entries from my table I can
still boot and still can use USB. The big difference is what shows up
in dmesg
[1.605000] s5p-ehci s5p-ehci: S5P EHCI Host Controller
Becomes:
[1.55] s5p-ehci 1211.usb: S5P EHCI Host Controller
...and some sysfs paths change.
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device
On Fri, Dec 21, 2012 at 12:14 AM, Vivek Gautam wrote: > On Wed, Dec 19, 2012 at 7:16 PM, Vivek Gautam > wrote: >> >> On Sat, Dec 15, 2012 at 12:50 PM, Grant Likely >> wrote: >>> On Thu, 13 Dec 2012 20:22:26 +0530, Vivek Gautam >>> wrote: Using chip specific compatible string as it should be. So fixing this for ehci-s5p, ohci-exynos and dwc3-exynos which till now used a generic 'exynos' in their compatible strings. This goes as per the discussion happened in the thread for [PATCH v2] ARM: Exynos5250: Enabling dwc3-exynos driver available at: http://www.spinics.net/lists/linux-usb/msg74145.html Vivek Gautam (2): usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device usb: dwc3-exynos: Fix compatible strings for the device >>> >>> for both patches: >>> Acked-by: Grant Likely >>> > > Any more thought about this patch-set? > Or does this change seems fine? These two changes look good to me. For both of them: Reviewed-by: Doug Anderson -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: phy: samsung: Add support to set pmu isolation
Vivek,
Nothing really serious below and things look good to me, but figured
I'd put a few nits in (sorry!).
On Fri, Dec 21, 2012 at 12:16 AM, Vivek Gautam wrote:
> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> index 7b26e2d..09f06f8 100644
> --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> @@ -9,3 +9,31 @@ Required properties:
> - compatible : should be "samsung,exynos4210-usbphy"
> - reg : base physical address of the phy registers and length of memory
> mapped
> region.
> +- #address-cells: should be 1.
> +- #size-cells: should be 0.
Doesn't match your example. Probably should be 1.
> diff --git a/drivers/usb/phy/samsung-usbphy.c
> b/drivers/usb/phy/samsung-usbphy.c
> index 5c5e1bb5..2260029 100644
> --- a/drivers/usb/phy/samsung-usbphy.c
> +++ b/drivers/usb/phy/samsung-usbphy.c
> /*
> + * struct samsung_usbphy_drvdata - driver data for various SoC variants
> + * @cpu_type: machine identifier
> + * @devphy_en_mask: device phy enable mask for PHY CONTROL register
> + * @hostphy_en_mask: host phy enable mask for PHY CONTROL register
> + *
> + * having different mask for host and device type phy
> + * helps in setting independent masks in case of SoCs like
> + * S5PV210 in which PHY0 and PHY1 enable bits belong to same
> + * register placed at [0] and [1] respectively.
> + * Although for newer SoCs like exynos these bits belong to
> + * different registers altogether placed at [0].
> + */
> +struct samsung_usbphy_drvdata {
> + int cpu_type;
> + int devphy_en_mask;
This is really a "devphy_dis_mask", isn't it? AKA: setting to 1
disables the phy and setting to 0 enables the phy.
> + int hostphy_en_mask;
Code below always uses devphy and only ever inits devphy. I assume
future code will init / use hostphy? Worth moving the hostphy part in
that patch?
> struct samsung_usbphy {
> struct usb_phy phy;
> @@ -81,12 +104,66 @@ struct samsung_usbphy {
> struct device *dev;
> struct clk *clk;
> void __iomem*regs;
> + void __iomem*phyctrl_pmureg;
> int ref_clk_freq;
> - int cpu_type;
> + struct samsung_usbphy_drvdata *drv_data;
nit: const
> +static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy)
> +{
> + struct device_node *usbphy_pmu;
> + u32 reg[2];
> + int ret;
> +
> + if (!sphy->dev->of_node) {
> + dev_err(sphy->dev, "Can't get usb-phy node\n");
> + return -ENODEV;
> + }
> +
> + usbphy_pmu = of_get_child_by_name(sphy->dev->of_node, "usbphy-pmu");
> + if (!usbphy_pmu)
> + dev_warn(sphy->dev, "Can't get usb-phy pmu control node\n");
> +
> + ret = of_property_read_u32_array(usbphy_pmu, "reg", reg, 2);
nit: use ARRAY_SIZE(reg)
> + if (!ret)
> + sphy->phyctrl_pmureg = ioremap(reg[0], reg[1]);
> +
> + of_node_put(usbphy_pmu);
> +
> + if (IS_ERR_OR_NULL(sphy->phyctrl_pmureg)) {
> + dev_err(sphy->dev, "Can't get usb-phy pmu control
> register\n");
I don't think there's any cases where it matters (you'll error out of
the driver if you return an error here), but seems like it might be
nice to set sphy->phyctrl_pmureg to NULL here since other places test
this member against NULL only.
> +static inline struct samsung_usbphy_drvdata
> +*samsung_usbphy_get_driver_data(struct platform_device *pdev)
> {
> if (pdev->dev.of_node) {
> const struct of_device_id *match;
> match = of_match_node(samsung_usbphy_dt_match,
> pdev->dev.of_node);
> - return (int) match->data;
> + return (struct samsung_usbphy_drvdata *) match->data;
nit: no need for a cast here, I believe.
> }
>
> - return platform_get_device_id(pdev)->driver_data;
> + return ((struct samsung_usbphy_drvdata *)
> + platform_get_device_id(pdev)->driver_data);
nit: no need for a cast here, I believe.
> +static struct samsung_usbphy_drvdata usbphy_s3c64xx = {
> + .cpu_type = TYPE_S3C64XX,
> + .devphy_en_mask = S3C64XX_USBPHY_ENABLE,
> +};
> +
> +static struct samsung_usbphy_drvdata usbphy_exynos4 = {
> + .cpu_type = TYPE_EXYNOS4210,
> + .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
> +};
> +
nit: static const for these structs?
-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 2/2] ASoC: SAMSUNG: Add DT support for i2s
Hi, On Wed, Dec 19, 2012 at 10:39 PM, Mark Brown wrote: > On Wed, Dec 19, 2012 at 01:24:14PM +, Grant Likely wrote: >> On Thu, 13 Dec 2012 16:12:53 +0530, Padmavathi Venna >> wrote: > >> > +- compatible : "samsung,samsung-i2s" > >> Isn't that kind of redundant? :-) > >> The format of the compatible strings should be ",-i2s". >> Please be specific about the part number that you're doing the binding >> for. For example; use "samsung,exynos4210-i2s" instead of >> "samsung,exynos-i2s". > > There are actually versioned IPs here (where the versions are used > publically in a few places) but it's not clearly documented which is > which. It would be reasonable to use the IP versions here I think. Samsung has three i2s drivers one for s3c24xx, one for s3c2412 and one for rest of the platforms. The above mentioned other platforms has Version 3/4/5 of i2s controllers. This dt binding is for for the i2s driver that has support for Version 3/4/5 of i2s controller. So "samsung,i2s-v5" is okay as compatible name? Please suggest me. Thanks Padma -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250
Hi Felipe,
On Wed, Dec 19, 2012 at 1:25 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Dec 19, 2012 at 11:52:01AM +0530, Vivek Gautam wrote:
>> >>> @@ -736,7 +1035,41 @@ static int __devinit samsung_usbphy_probe(struct
>> >>> platform_device *pdev)
>> >>>
>> >>> sphy->clk = clk;
>> >>>
>> >>> - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
>> >>> + sphy->has_usb3 = (sphy->cpu_type == TYPE_EXYNOS5250);
>> >>> +
>> >>> + if (sphy->has_usb3) {
>> >>> + struct resource *usb3phy_mem;
>> >>> + void __iomem*usb3phy_base;
>> >>> +
>> >>> + usb3phy_mem = platform_get_resource(pdev, IORESOURCE_MEM,
>> >>> 1);
>> >>> + if (!usb3phy_mem) {
>> >>> + dev_err(dev, "%s: missing mem resource\n",
>> >>> __func__);
>> >>> + return -ENODEV;
>> >>> + }
>> >>> +
>> >>> + usb3phy_base = devm_request_and_ioremap(dev, usb3phy_mem);
>> >>> + if (!usb3phy_base) {
>> >>> + dev_err(dev, "%s: register mapping failed\n",
>> >>> __func__);
>> >>> + return -ENXIO;
>> >>> + }
>> >>> +
>> >>> + sphy->usb3phy.regs_phy = usb3phy_base;
>> >>> + sphy->usb3phy.phy.dev = sphy->dev;
>> >>> + sphy->usb3phy.phy.label = "samsung-usb3phy";
>> >>> + sphy->usb3phy.phy.init = samsung_usb3phy_init;
>> >>> + sphy->usb3phy.phy.shutdown = samsung_usb3phy_shutdown;
>> >>> + }
>> >>> +
>> >>> + ret = usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
>> >>> + if (ret)
>> >>> + return ret;
>> >>
>> >> is this realy how your HW behaves ? USB2 and USB3 phys are a single HW
>> >> entity ? I kinda doubt that :-s
>> >>
>>
>> They are separate HW in fact.
>> So, do you expect to see a separate driver interface for USB 3.0 type phy ?
>
> yes. Just as we did on OMAP. One driver for the USB2 part and one driver
> for USB3 part (which are actually two, but you can only talk to them as
> one) :-)
>
Sure as you suggest, we will pull out the USB 3.0 code from here and put
a new driver in place for the same.
>> That will be quite similar architecturally to current samsung-usbphy
>> driver for USB 2.0 type phy,
>> and may require some code duplication too.
>
> If it duplicates code, then perhaps it's best to keep it as is but I'm
> actually surprised you guys have similar programming model on both
> parts. I mean, the differences at HW behavior are huge: on one side you
> use ULPI/UTMI+ on the other PIPE3, on one side you have 480Mbps
> half-duplex signalling, on the other you have 5Gbps dual simplex
> signalling, the differences go on and on.
>
The programming model for USB phy is just about setting up the phy registers
for example to provide proper clocks to PHY controller, setting up setting up
the UTMI block etc, under a sequence.
> Also, what you say about duplicating, it seems to me that it will
> duplicate only the boilerplate part (allocating memory, having a
> platform_driver, and so on), because you _do_ have completely separate
> functions to handle usb3 part.
>
Yes, the duplication was in fact just the boilerplate part ;-)
apart from having separate functions for USB 3.0 type phy.
> One more comment below:
>
>> +static u32 exynos5_usb3phy_set_clock(struct samsung_usbphy *sphy)
>> +{
>> + u32 reg;
>> + u32 refclk;
>> +
>> + refclk = sphy->ref_clk_freq;
>> +
>> + reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
>> + PHYCLKRST_FSEL(refclk);
>> +
>> + switch (refclk) {
>> + case HOST_CTRL0_FSEL_CLKSEL_50M:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x00));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_20M:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x00));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_19200K:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x88));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_24M:
>> + default:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x88));
>> + break;
>> + }
>> +
>> + return reg;
>> +}
>
> looks like this should be done by commong clock framework by clock
> reparenting perhaps ?
>
Need to check on this one.
> --
> balbi
--
Thanks & Regards
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] ARM: Exynos5250: Enabling ehci-s5p driver
Hi all,
On Wed, Dec 19, 2012 at 7:20 PM, Vivek Gautam wrote:
> CC: Doug Anderson
>
>
> On Sat, Dec 15, 2012 at 12:53 PM, Grant Likely
> wrote:
>> On Thu, 13 Dec 2012 22:06:01 +0530, Vivek Gautam
>> wrote:
>>> Adding EHCI device tree node for Exynos5250 along with
>>> the device base adress and gpio line for vbus.
>>>
>>> Signed-off-by: Vivek Gautam
>>> Acked-by: Jingoo Han
>>> ---
>>> .../devicetree/bindings/usb/exynos-usb.txt | 25
>>>
>>> arch/arm/boot/dts/exynos5250-smdk5250.dts |4 +++
>>> arch/arm/boot/dts/exynos5250.dtsi |6
>>> arch/arm/mach-exynos/include/mach/map.h|1 +
>>> arch/arm/mach-exynos/mach-exynos5-dt.c |2 +
>>> 5 files changed, 38 insertions(+), 0 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/usb/exynos-usb.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
>>> b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>>> new file mode 100644
>>> index 000..e8bbb47
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
>>> @@ -0,0 +1,25 @@
>>> +Samsung Exynos SoC USB controller
>>> +
>>> +The USB devices interface with USB controllers on Exynos SOCs.
>>> +The device node has following properties.
>>> +
>>> +EHCI
>>> +Required properties:
>>> + - compatible: should be "samsung,exynos4210-ehci" for USB 2.0
>>> + EHCI controller in host mode.
>>> + - reg: physical base address of the controller and length of memory mapped
>>> + region.
>>> + - interrupts: interrupt number to the cpu.
>>> +
>>> +Optional properties:
>>> + - samsung,vbus-gpio: if present, specifies the GPIO that
>>> + needs to be pulled up for the bus to be powered.
>>> +
>>> +Example:
>>> +
>>> + usb@1211 {
>>> + compatible = "samsung,exynos4210-ehci";
>>> + reg = <0x1211 0x100>;
>>> + interrupts = <0 71 0>;
>>> + samsung,vbus-gpio = <&gpx2 6 1 3 3>;
>>> + };
>>> diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts
>>> b/arch/arm/boot/dts/exynos5250-smdk5250.dts
>>> index 711b55f..f990086 100644
>>> --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
>>> +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
>>> @@ -218,4 +218,8 @@
>>> i2s_2: i2s@12D7 {
>>> status = "disabled";
>>> };
>>> +
>>> + usb@1211 {
>>> + samsung,vbus-gpio = <&gpx2 6 1 3 3>;
>>> + };
>>> };
>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>>> b/arch/arm/boot/dts/exynos5250.dtsi
>>> index 581e57a..584bb9a 100644
>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>> @@ -299,6 +299,12 @@
>>> rx-dma-channel = <&pdma0 11>; /* preliminary */
>>> };
>>>
>>> + usb@1211 {
>>> + compatible = "samsung,exynos4210-ehci";
>>> + reg = <0x1211 0x100>;
>>> + interrupts = <0 71 0>;
>>> + };
>>> +
>>> amba {
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h
>>> b/arch/arm/mach-exynos/include/mach/map.h
>>> index cbb2852..b2c662f 100644
>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>> @@ -201,6 +201,7 @@
>>> #define EXYNOS4_PA_EHCI 0x1258
>>> #define EXYNOS4_PA_OHCI 0x1259
>>> #define EXYNOS4_PA_HSPHY 0x125B
>>> +#define EXYNOS5_PA_EHCI 0x1211
>>> #define EXYNOS4_PA_MFC 0x1340
>>>
>>> #define EXYNOS4_PA_UART 0x1380
>>> diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c
>>> b/arch/arm/mach-exynos/mach-exynos5-dt.c
>>> index 462e5ac..b3b9af1 100644
>>> --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
>>> +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
>>> @@ -110,6 +110,8 @@ static const struct of_dev_auxdata
>>> exynos5250_auxdata_lookup[] __initconst = {
>>> "samsung-i2s.1", NULL),
>>> OF_DEV_AUXDATA("samsung,samsung-i2s", 0x12D7,
>>> "samsung-i2s.2", NULL),
>>> + OF_DEV_AUXDATA("samsung,exynos4210-ehci", EXYNOS5_PA_EHCI,
>>> + "s5p-ehci", NULL),
>>
>> I'm assuming the above change is temporary. What is left to be done to
>> drop the auxdata in theses two patches?
>>
>> Otherwise the patch looks fine.
>>
>> Acked-by: Grant Likely
Any more thought about this patch?
Or does this change seems fine?
>> --
>> 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/
>
>
>
> --
> Thanks & Regards
> Vivek
--
Thanks & Regards
Vivek
--
To unsubscribe from this list: send the line "un
Re: [PATCH 0/2] usb: exynos: Fix compatible strings used for device
Hi all, On Wed, Dec 19, 2012 at 7:16 PM, Vivek Gautam wrote: > CC: Doug Anderson > > > On Sat, Dec 15, 2012 at 12:50 PM, Grant Likely > wrote: >> On Thu, 13 Dec 2012 20:22:26 +0530, Vivek Gautam >> wrote: >>> Using chip specific compatible string as it should be. >>> So fixing this for ehci-s5p, ohci-exynos and dwc3-exynos >>> which till now used a generic 'exynos' in their compatible strings. >>> >>> This goes as per the discussion happened in the thread for >>> [PATCH v2] ARM: Exynos5250: Enabling dwc3-exynos driver >>> available at: >>> http://www.spinics.net/lists/linux-usb/msg74145.html >>> >>> Vivek Gautam (2): >>> usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device >>> usb: dwc3-exynos: Fix compatible strings for the device >> >> for both patches: >> Acked-by: Grant Likely >> Any more thought about this patch-set? Or does this change seems fine? >>> >>> drivers/usb/dwc3/dwc3-exynos.c |2 +- >>> drivers/usb/host/ehci-s5p.c|2 +- >>> drivers/usb/host/ohci-exynos.c |2 +- >>> 3 files changed, 3 insertions(+), 3 deletions(-) >>> >>> -- >>> 1.7.6.5 >>> >> >> -- >> Grant Likely, B.Sc, P.Eng. >> Secret Lab Technologies, Ltd. >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Thanks & Regards > Vivek -- Thanks & Regards Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usb: phy: samsung: Add support to set pmu isolation
Adding support to parse device node data in order to get
required properties to set pmu isolation for usb-phy.
Signed-off-by: Vivek Gautam
---
.../devicetree/bindings/usb/samsung-usbphy.txt | 28
drivers/usb/phy/samsung-usbphy.c | 145 +---
2 files changed, 152 insertions(+), 21 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 7b26e2d..09f06f8 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -9,3 +9,31 @@ Required properties:
- compatible : should be "samsung,exynos4210-usbphy"
- reg : base physical address of the phy registers and length of memory mapped
region.
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Optional properties:
+- The samsung usbphy nodes should provide the following information required
+ by USB-phy controller to enable/disable the phy controller.
+ - reg : base physical address of PHY control register in PMU which
+ enables/disables the phy controller.
+ The size of this register is the total sum of size of all
phy-control
+ registers that the SoC has. For example, the size will be
+ '0x4' in case we have only one phy-control register (like in
S3C64XX) or
+ '0x8' in case we have two phy-control registers (like in Exynos4210)
+ and so on.
+
+Example:
+ - Exysno4210
+
+ usbphy@125B {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "samsung,exynos4210-usbphy";
+ reg = <0x125B 0x100>;
+
+ usbphy-pmu {
+ /* USB device and host PHY_CONTROL registers */
+ reg = <0x10020704 0x8>;
+ };
+ };
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index 5c5e1bb5..2260029 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -60,20 +60,43 @@
#define MHZ (1000*1000)
#endif
+#define S3C64XX_USBPHY_ENABLE (0x1 << 16)
+#define EXYNOS_USBPHY_ENABLE (0x1 << 0)
+
enum samsung_cpu_type {
TYPE_S3C64XX,
TYPE_EXYNOS4210,
};
/*
+ * struct samsung_usbphy_drvdata - driver data for various SoC variants
+ * @cpu_type: machine identifier
+ * @devphy_en_mask: device phy enable mask for PHY CONTROL register
+ * @hostphy_en_mask: host phy enable mask for PHY CONTROL register
+ *
+ * having different mask for host and device type phy
+ * helps in setting independent masks in case of SoCs like
+ * S5PV210 in which PHY0 and PHY1 enable bits belong to same
+ * register placed at [0] and [1] respectively.
+ * Although for newer SoCs like exynos these bits belong to
+ * different registers altogether placed at [0].
+ */
+struct samsung_usbphy_drvdata {
+ int cpu_type;
+ int devphy_en_mask;
+ int hostphy_en_mask;
+};
+
+/*
* struct samsung_usbphy - transceiver driver state
* @phy: transceiver structure
* @plat: platform data
* @dev: The parent device supplied to the probe function
* @clk: usb phy clock
* @regs: usb phy register memory base
+ * @phyctrl_pmureg: usb device phy-control pmu register memory base
* @ref_clk_freq: reference clock frequency selection
- * @cpu_type: machine identifier
+ * @drv_data: driver data available for different cpu types
*/
struct samsung_usbphy {
struct usb_phy phy;
@@ -81,12 +104,66 @@ struct samsung_usbphy {
struct device *dev;
struct clk *clk;
void __iomem*regs;
+ void __iomem*phyctrl_pmureg;
int ref_clk_freq;
- int cpu_type;
+ struct samsung_usbphy_drvdata *drv_data;
};
#define phy_to_sphy(x) container_of((x), struct samsung_usbphy, phy)
+static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy)
+{
+ struct device_node *usbphy_pmu;
+ u32 reg[2];
+ int ret;
+
+ if (!sphy->dev->of_node) {
+ dev_err(sphy->dev, "Can't get usb-phy node\n");
+ return -ENODEV;
+ }
+
+ usbphy_pmu = of_get_child_by_name(sphy->dev->of_node, "usbphy-pmu");
+ if (!usbphy_pmu)
+ dev_warn(sphy->dev, "Can't get usb-phy pmu control node\n");
+
+ ret = of_property_read_u32_array(usbphy_pmu, "reg", reg, 2);
+ if (!ret)
+ sphy->phyctrl_pmureg = ioremap(reg[0], reg[1]);
+
+ of_node_put(usbphy_pmu);
+
+ if (IS_ERR_OR_NULL(sphy->phyctrl_pmureg)) {
+ dev_err(sphy->dev, "Can't get usb-phy pmu control register\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * Set isolation here for phy.
+ * SOCs control this by controlling corresponding PMU registers
+ */
+static void samsung_usbphy_set_iso
[PATCH v3] usb: phy: samsung: Add support to set pmu isolation
Changes from v2: - Removed the phandle type of device node properties, instead using sub-nodes now. - Removed the property 'samsung,enable-mask' since it is SoC dependent (SoCs like S5PV210 and S3C64XX have different bits to enable/disable phy controller in comparison to exysno4210 onwards). - Maintaining the phy enable mask (which is SoC dependent) for device type phy and host type phy in the driver data. - Re-structuring to get device properties using sub-nodes for 'usbphy-pmu' Changes from v1: - Changed the name of property for phy handler from'samsung,usb-phyctrl' to 'samsung,usb-phyhandle' to make it look more generic. - Similarly 'samsung,phyctrl-reg' is changed to 'samsung,phyhandle-reg' - Added a check for 'samsung,usb-phyhandle' before getting node from phandle. - Putting the node using 'of_node_put()' which had been missed. - Adding necessary check for the pointer in 'samsung_usbphy_set_isolation()' to avoid any NULL pointer dereferencing. - Unmapping the register ioremapped in 'samsung_usbphy_parse_dt_param()'. Based on usb-next branch with Praveen Paneri's patches on top of it. - http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/134476.html - http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/131763.html Vivek Gautam (1): usb: phy: samsung: Add support to set pmu isolation .../devicetree/bindings/usb/samsung-usbphy.txt | 28 drivers/usb/phy/samsung-usbphy.c | 145 +--- 2 files changed, 152 insertions(+), 21 deletions(-) -- 1.7.6.5 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
