LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 6/6][v3] perf: Document the ABI of perf sysfs entries
From: Sukadev Bhattiprolu @ 2013-01-16 18:58 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Andi Kleen, Peter Zijlstra, robert.richter, Greg KH,
	Anton Blanchard, linux-kernel, Stephane Eranian, linuxppc-dev,
	Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo
In-Reply-To: <20130116113013.GA1042@krava.brq.redhat.com>

Jiri Olsa [jolsa@redhat.com] wrote:
| On Tue, Jan 15, 2013 at 03:57:59PM -0300, Arnaldo Carvalho de Melo wrote:
| > Em Wed, Jan 09, 2013 at 05:07:03PM -0800, Sukadev Bhattiprolu escreveu:
| > > [PATCH 6/6][v3] perf: Document the ABI of perf sysfs entries
| > > 
| > > This patchset addes two new sets of files to sysfs:
| > > 
| > > 	- generic and POWER-specific perf events in /sys/devices/cpu/events/
| > > 	- perf event config format in /sys/devices/cpu/format/event
| > > 
| > > Document the format of these files which would become part of the ABI.
| > > 
| > > Changelog[v3]:
| > > 	[Greg KH] Include ABI documentation.
| > 
| > Jiri, can I have your ack on this one too?
| 
| hm, we already already 'format' definition but in testing section
| (and we are missing 'events' definition there.. my bad)

I see that file now ! I have removed that documentation from my patch.
| 
| anyway, after reading Documentation/ABI/README looks like we
| should go for 'testing' section with this as well.. stable
| seems scary at this point ;-)

Agree.

Here is the updated patch.
---

This patchset addes two new sets of files to sysfs for POWER architecture.

	- perf event config format in /sys/devices/cpu/format/event
	- generic and POWER-specific perf events in /sys/devices/cpu/events/

The format of the first file is already documented in:

	Documentation/ABI/testing/sysfs-bus-event_source-devices-format

Document the format of the second set of files '/sys/devices/cpu/events/*'
which would also become part of the ABI.

Changelog[v3.1]:
	(small changes to this one patch).
	[Jiri Olsa]: Remove the documentation for the 'config format' file
	as it is already documented.
	[Jiri Olsa]: Move the documentation of 'events' also to 'testing/'

Changelog[v3]:
	[Greg KH] Include ABI documentation.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 .../testing/sysfs-bus-event_source-devices-events  |   58 ++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletions(-)
 delete mode 100644 Documentation/ABI/stable/sysfs-devices-cpu-events
 create mode 100644 Documentation/ABI/testing/sysfs-bus-event_source-devices-events

diff --git a/Documentation/ABI/stable/sysfs-devices-cpu-events b/Documentation/ABI/stable/sysfs-devices-cpu-events
deleted file mode 100644
index e69de29..0000000
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-events b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
new file mode 100644
index 0000000..2c4081e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-events
@@ -0,0 +1,58 @@
+What:		/sys/devices/cpu/events/
+		/sys/devices/cpu/events/branch-misses
+		/sys/devices/cpu/events/cache-references
+		/sys/devices/cpu/events/cache-misses
+		/sys/devices/cpu/events/stalled-cycles-frontend
+		/sys/devices/cpu/events/branch-instructions
+		/sys/devices/cpu/events/stalled-cycles-backend
+		/sys/devices/cpu/events/instructions
+		/sys/devices/cpu/events/cpu-cycles
+
+Date:		2012/01/NN
+
+Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
+
+Description:	Generic performance monitoring events
+
+		A collection of performance monitoring events that may be
+		supported by many/most CPUs. These events can be monitored
+		using the 'perf(1)' tool.
+
+		The contents of each file would look like:
+
+			event=0xNNNN
+
+		where 'N' is a hex digit and the number '0xNNNN' shows the
+		"raw code" for the perf event identified by the file's
+		"basename".
+
+
+What: 		/sys/devices/cpu/events/PM_LD_MISS_L1
+		/sys/devices/cpu/events/PM_LD_REF_L1
+		/sys/devices/cpu/events/PM_CYC
+		/sys/devices/cpu/events/PM_BRU_FIN
+		/sys/devices/cpu/events/PM_GCT_NOSLOT_CYC
+		/sys/devices/cpu/events/PM_BRU_MPRED
+		/sys/devices/cpu/events/PM_INST_CMPL
+		/sys/devices/cpu/events/PM_CMPLU_STALL
+
+Date:		2013/01/08
+
+Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
+		Linux Powerpc mailing list <linuxppc-dev@ozlabs.org>
+
+Description:	POWER-systems specific performance monitoring events
+
+		A collection of performance monitoring events that may be
+		supported by the POWER CPU. These events can be monitored
+		using the 'perf(1)' tool.
+
+		These events may not be supported by other CPUs.
+
+		The contents of each file would look like:
+
+			event=0xNNNN
+
+		where 'N' is a hex digit and the number '0xNNNN' shows the
+		"raw code" for the perf event identified by the file's
+		"basename".
-- 
1.7.1

^ permalink raw reply related

* Re: [RFC PATCH v2 00/12] System device hot-plug framework
From: Rafael J. Wysocki @ 2013-01-17  0:50 UTC (permalink / raw)
  To: Toshi Kani
  Cc: linux-s390, jiang.liu, wency, linux-acpi, gregkh, linux-kernel,
	linux-mm, isimatu.yasuaki, yinghai, srivatsa.bhat, guohanjun,
	bhelgaas, akpm, linuxppc-dev, lenb
In-Reply-To: <1357861230-29549-1-git-send-email-toshi.kani@hp.com>

On Thursday, January 10, 2013 04:40:18 PM Toshi Kani wrote:
> This patchset is a prototype of proposed system device hot-plug framework
> for design review.  Unlike other hot-plug environments, such as USB and
> PCI, there is no common framework for system device hot-plug [1].
> Therefore, this patchset is designed to provide a common framework for
> hot-plugging and online/offline operations of system devices, such as CPU,
> Memory and Node.  While this patchset only supports ACPI-based hot-plug
> operations, the framework itself is designed to be platform-neural and
> can support other FW architectures as necessary.
> 
> This patchset is based on Linus's tree (3.8-rc3).
> 
> I have seen a few stability issues with 3.8-rc3 in my testing and will
> look into their solutions.
> 
> [1] System device hot-plug frameworks for ppc and s390 are implemented
>     for specific platforms and products.
> 
> 
> Background: System Device Initialization
> ========================================
> System devices, such as CPU and memory, must be initialized during early
> boot sequence as they are the essential components to provide low-level
> services, ex. scheduling, memory allocation and interrupts, which are
> the foundations of the kernel services.  start_kernel() and kernel_init()
> manage the boot-up sequence to initialize system devices and low-level
> services in pre-defined order as shown below. 
> 
>   start_kernel()
>     boot_cpu_init()          // init cpu0
>     setup_arch()
>       efi_init()             // init EFI memory map
>       initmem_init()         // init NUMA
>       x86_init.paging.pagetable_init() // init page table
>       acpi_boot_init()       // parse ACPI MADT table
>         :
>   kernel_init()
>     kernel_init_freeable()
>       smp_init()             // init other CPUs
>         :
>       do_basic_setup()
>         driver_init()
>           cpu_dev_init()     // build system/cpu tree
>           memory_dev_init()  // build system/memory tree
>         do_initcalls()
>           acpi_init()        // build ACPI device tree
> 
> Note that drivers are initialized at the end of the boot sequence as they
> depend on the kernel services from system devices.  Hence, while system
> devices may be exposed to sysfs with their pseudo drivers, their
> initialization may not be fully integrated into the driver structures.  
> 
> Overview of the System Device Hot-plug Framework
> ================================================
> Similar to the boot-up sequence, the system device hot-plug framework
> provides a sequencer that calls all registered handlers in pre-defined
> order for hot-add and hot-delete of system devices.  It allows any modules
> initializing system devices in the boot-up sequence to participate in
> the hot-plug operations as well.  In high-level, there are two types of
> handlers, 1) FW-dependent (ex. ACPI) handlers that enumerate or eject
> system devices, and 2) system device (ex. CPU, Memory) management handlers
> that online or offline the enumerated system devices.  Online/offline
> operations are sub-set of hot-add/delete operations.  The ordering of the
> handlers are symmetric between hot-add (online) and hot-delete (offline)
> operations.
> 
>         hot-add    online
>            |    ^    :    ^
>   HW Enum/ |    |    :    :
>     Eject  |    |    :    :
>            |    |    :    :
>   Online/  |    |    |    |
>   Offline  |    |    |    |
>            V    |    V    |
>              hot-del   offline
> 
> The handlers may not call other handlers directly to exceed their role.
> Therefore, the role of the handlers in their modules remains consistent
> with their role at the boot-up sequence.  For instance, the ACPI module
> may not perform online or offline of system devices.
> 
> System Device Hot-plug Operation
> ================================
> 
> Serialized Startup
> ------------------
> The framework provides an interface (hp_submit_req) to request a hot-plug
> operation.  All requests are queued to and run on a single work queue.
> The framework assures that there is only a single hot-plug or online/
> offline operation running at a time.  A single request may however target
> to multiple devices.  This makes the execution context of handlers to be
> consistent with the boot-up sequence and enables code sharing.
> 
> Phased Execution
> ----------------
> The framework proceeds hot-plug and online/offline operations in the 
> following three phases.  The modules can register their handlers to each
> phase.  The framework also initiates a roll-back operation if any hander
> failed in the validate or execute phase.
> 
> 1) Validate Phase - Handlers validate if they support a given request
> without making any changes to target device(s).  They check any known
> restrictions and/or prerequisite conditions to their modules, and fail
> an unsupported request before making any changes.  For instance, the
> memory module may check if a hot-remove request is targeted to movable
> ranges.
> 
> 2) Execute Phase - Handlers make requested change within the scope that
> its roll-back is possible in case of a failure.  Execute handlers must
> implement their roll-back procedures.
> 
> 3) Commit Phase - Handlers make the final change that cannot be rolled-back.
> For instance, the ACPI module invokes _EJ0 for a hot-remove operation.
> 
> System Device Management Modules
> ================================
> 
> CPU Handlers
> ------------
> CPU handlers are provided by the CPU driver in drivers/base/cpu.c, and
> perform CPU online/offline procedures when CPU device(s) is added or
> deleted during an operation.
> 
> Memory Handlers
> ---------------
> Memory handlers are provided by the memory module in mm/memory_hotplug.c,
> and perform Memory online/offline procedure when memory device(s) is
> added or deleted during an operation.
> 
> FW-dependent Modules
> ====================
> 
> ACPI Bus Handlers
> -----------------
> ACPI bus handlers are provided by the ACPI core in drivers/acpi/bus.c,
> and construct/destruct acpi_device object(s) during a hot-plug operation.
> 
> ACPI Resource Handlers
> ----------------------
> ACPI resource handlers are provided by the ACPI core in
> drivers/acpi/hp_resource.c, and set device resource information to
> a request during a hot-plug operation.  This device resource information
> is then consumed by the system device management modules for their
> online/offline procedure.
> 
> ACPI Drivers
> ------------
> ACPI drivers are called from the ACPI core during a hot-plug operation
> through the following interfaces.  ACPI drivers are not called from the
> framework directly, and remain internal to the ACPI core.  ACPI drivers
> may not initiate online/offline of a device.
> 
> .add - Construct device-specific information to a given acpi_device.
> Called at boot, hot-add and sysfs bind.
> 
> .remove - Destruct device-specific information to a given acpi_device.
> Called at hot-remove and sysfs unbind.
> 
> .resource - Set device-specific resource information to a given hot-plug
> request.  Called at hot-add and hot-remove.

At this point I'd like to clearly understand how the code is supposed to work.

>From what I can say at the moment it all boils down to having two (ordered)
lists of notifiers (shp_add_list, shp_del_list) that can be added to or removed
from with shp_register_handler() and shp_unregister_handler(), respectively
(BTW, the abbreviation "hdr" makes me think about a "header" rather than a
"handler", but maybe that's just me :-)), and a workqueue for requests (why do
we need a separate workqueue for that?).

Whoever needs to carry out a hotplug operation is supposed to prepare a request
and then put it into the workqueue with shp_submit_request().  The framework
will then execute all of the notifier callbacks from the appropriate notifier
list (depending on whether the operation is a hot-add or a hot-remove).  If any
of those callbacks returns an error code and it is not too late (the order of
the failing notifier is not too high), the already executed notifier callbacks
will be run again with the "rollback" argument set to 1 (why not to use bool?)
to indicate that they are supposed to bring things back to the initial state.
Error codes returned in that stage only cause messages to be printed.

Is the description above correct?

If so, it looks like subsystems are supposed to register notifiers (handlers)
for hotplug/hot-remove operations of the devices they handle.  They are
supposed to use predefined order values to indicate what kinds of devices
those are.  Then, hopefully, if they do everything correctly, and the
initiator of a hotplug/hot-remove operation prepares the request correctly,
the callbacks will be executed in the right order, they will find their
devices in the list attached to the request object and they will do what's
necessary with them.

Am I still on the right track?

If that's the case, I have a few questions.

(1) Why is this limited to system devices?

(2) What's the guarantee that the ordering of hot-removal (for example) of CPU
    cores with respect to memory and host bridges will always be the same?
    What if the CPU cores themselves need to be hot-removed in a specific
    order?

(3) What's the guarantee that the ordering of shp_add_list and shp_del_list
    will be in agreement with the ordering of the device hierarchy?

(4) Why do you think that the ordering of hot-plug operations needs to be
    independent of the device herarchy ordering?

(5) Why do you think it's a good idea to require every callback routine to
    browse the entire list of devices attached to the request object?  Wouldn't
    it be more convenient if they were called only for the types of devices
    they have declared to handle?  [That would reduce some code duplication,
    for example.]

(6) Why is it convenient to use order values (priorities) of notifiers to
    indicate both the ordering with respect to the other notifiers and the
    "level" (e.g. whether or not rollback is possible) at the same time?  Those
    things appear to be conceptually distinct.

(7) Why callbacks used for "add" operations still need to check if the
    operation type is "add" (cpu_add_execute() does that for example)?

(8) What problems *exactly* this is supposed to address?  Can you give a few
    examples, please?

I guess I'll have more questions going forward.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [RFC PATCH 3/6] usb: otg: utils: change the phy lib to support multiple PHYs of same type
From: Simon Horman @ 2013-01-17  0:31 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, haojian.zhuang,
	linux-omap, linux-arm-kernel, eric.y.miao, b-cousson, gregkh,
	linux-usb, linux-kernel, balbi, cbou, rob, dwmw2
In-Reply-To: <1358348462-27693-4-git-send-email-kishon@ti.com>

On Wed, Jan 16, 2013 at 08:30:59PM +0530, Kishon Vijay Abraham I wrote:
> In order to add support for multipe PHY's of the same type, the API's
> for adding PHY and getting PHY has been changed. Now the binding
> information of the PHY and controller should be done in platform file
> using usb_bind_phy API. And for getting a PHY, the device pointer of the
> USB controller and an index should be passed. Based on the binding
> information that is added in the platform file, get_phy will return the
> approappropriate PHY.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  arch/arm/mach-shmobile/board-marzen.c |    2 +-

Modification to the above file:

Acked-by: Simon Horman <horms+renesas@verge.net.au>

^ permalink raw reply

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-17  9:14 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, balbi, kernel, Shawn Guo, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20130116014824.GD16514@nchen-desktop>

[-- Attachment #1: Type: text/plain, Size: 6058 bytes --]

On Wed, Jan 16, 2013 at 09:48:25AM +0800, Peter Chen wrote:
> On Tue, Jan 15, 2013 at 10:03:46PM +0800, Shawn Guo wrote:
> > On Tue, Jan 15, 2013 at 10:29:33AM +0800, Peter Chen wrote:
> > > As mach/hardware.h is deleted, we need to use platform_device_id to
> > > differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
> > > already used pdata to differentiate runtime
> > > 
> > > Meanwhile we update the platform code accordingly.
> > > 
> > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > ---
> > >  arch/arm/mach-imx/devices/devices-common.h        |    1 +
> > >  arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++---
> > >  drivers/usb/gadget/fsl_mxc_udc.c                  |   24 +++++-------
> > >  drivers/usb/gadget/fsl_udc_core.c                 |   42 +++++++++++++--------
> > >  4 files changed, 45 insertions(+), 37 deletions(-)
> > 
> > Since we are splitting the original patch anyway, it's a bit strange
> > to me that you are mixing arch/arm/mach-imx and drivers/usb/gadget
> > in this patch.  I'm fine with it, since I assume all the patches to
> > go via USB tree anyway.
> > 
> > > 
> > > diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
> > > index 6277baf..9bd5777 100644
> > > --- a/arch/arm/mach-imx/devices/devices-common.h
> > > +++ b/arch/arm/mach-imx/devices/devices-common.h
> > > @@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
> > >  
> > >  #include <linux/fsl_devices.h>
> > >  struct imx_fsl_usb2_udc_data {
> > > +	const char *devid;
> > >  	resource_size_t iobase;
> > >  	resource_size_t irq;
> > >  };
> > > diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > index 37e4439..fb527c7 100644
> > > --- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > +++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > @@ -11,35 +11,36 @@
> > >  #include "../hardware.h"
> > >  #include "devices-common.h"
> > >  
> > > -#define imx_fsl_usb2_udc_data_entry_single(soc)				\
> > > +#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)			\
> > >  	{								\
> > > +		.devid = _devid,					\
> > >  		.iobase = soc ## _USB_OTG_BASE_ADDR,			\
> > >  		.irq = soc ## _INT_USB_OTG,				\
> > >  	}
> > >  
> > >  #ifdef CONFIG_SOC_IMX25
> > >  const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
> > > -	imx_fsl_usb2_udc_data_entry_single(MX25);
> > > +	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
> > >  #endif /* ifdef CONFIG_SOC_IMX25 */
> > >  
> > >  #ifdef CONFIG_SOC_IMX27
> > >  const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
> > > -	imx_fsl_usb2_udc_data_entry_single(MX27);
> > > +	imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
> > >  #endif /* ifdef CONFIG_SOC_IMX27 */
> > >  
> > >  #ifdef CONFIG_SOC_IMX31
> > >  const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
> > > -	imx_fsl_usb2_udc_data_entry_single(MX31);
> > > +	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
> > >  #endif /* ifdef CONFIG_SOC_IMX31 */
> > >  
> > >  #ifdef CONFIG_SOC_IMX35
> > >  const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
> > > -	imx_fsl_usb2_udc_data_entry_single(MX35);
> > > +	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
> > >  #endif /* ifdef CONFIG_SOC_IMX35 */
> > >  
> > >  #ifdef CONFIG_SOC_IMX51
> > >  const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
> > > -	imx_fsl_usb2_udc_data_entry_single(MX51);
> > > +	imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
> > >  #endif
> > >  
> > >  struct platform_device *__init imx_add_fsl_usb2_udc(
> > > @@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
> > >  			.flags = IORESOURCE_IRQ,
> > >  		},
> > >  	};
> > > -	return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
> > > +	return imx_add_platform_device_dmamask(data->devid, -1,
> > >  			res, ARRAY_SIZE(res),
> > >  			pdata, sizeof(*pdata), DMA_BIT_MASK(32));
> > >  }
> > 
> > <snip>
> > 
> > > +static const struct platform_device_id fsl_udc_devtype[] = {
> > > +	{
> > > +		.name = "imx-udc-mx25",
> > > +	}, {
> > > +		.name = "imx-udc-mx27",
> > > +	}, {
> > > +		.name = "imx-udc-mx31",
> > > +	}, {
> > > +		.name = "imx-udc-mx35",
> > > +	}, {
> > > +		.name = "imx-udc-mx51",
> > > +	}
> > > +};
> > 
> > From what I understand balbi's comment, he dislikes this full list of
> > device id.  Instead, he prefers to something like below.
> > 
> > static const struct platform_device_id fsl_udc_devtype[] = {
> > 	{
> > 		.name = "imx-udc-mx27",
> > 	}, {
> > 		.name = "imx-udc-mx51",
> > 	}
> > };
> > 
> > It basically tells that we are handling two type of devices here, one
> > is imx-udc-mx27 type and the other is imx-udc-mx51 type, with mx25/31/35
> > completely compatible with mx27 type.  We choose mx27 instead of mx25
> > to define the type because mx27 Si came out earlier than mx25.  That
> > said, we generally choose the earlies SoC name to define a particular
> > version of IP block, since hardware version is mostly unavailable or
> > unreliable.
> > 
> > But that also means in platform code which create the platform_device,
> > you will need to use name "imx-udc-mx27" for even mx25/31/35.
> > 
> > 	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
> > 	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
> > 	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
> > 
> > Considering this is a piece of code we will not use for any new
> > hardware, I'm fine with either way.
> > 
> > So, balbi, it's all your call to accept the series as it is or ask for
> > another iteration.

right :-)

> Thanks Shawn. Let's see Felipe's comment, nevertheless, I will send v6 patch
> due to a compile error at mx25

Shawn is right.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-17  9:28 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: r58472, gregkh, linux-usb, kernel, Shawn Guo, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20130117091422.GF10814@arwen.pp.htv.fi>

On Thu, Jan 17, 2013 at 11:14:22AM +0200, Felipe Balbi wrote:
> On Wed, Jan 16, 2013 at 09:48:25AM +0800, Peter Chen wrote:
> > On Tue, Jan 15, 2013 at 10:03:46PM +0800, Shawn Guo wrote:
> > > On Tue, Jan 15, 2013 at 10:29:33AM +0800, Peter Chen wrote:
> > > > As mach/hardware.h is deleted, we need to use platform_device_id to
> > > > differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
> > > > already used pdata to differentiate runtime
> > > > 
> > > > Meanwhile we update the platform code accordingly.
> > > > 
> > > > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > > > ---
> > > >  arch/arm/mach-imx/devices/devices-common.h        |    1 +
> > > >  arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++---
> > > >  drivers/usb/gadget/fsl_mxc_udc.c                  |   24 +++++-------
> > > >  drivers/usb/gadget/fsl_udc_core.c                 |   42 +++++++++++++--------
> > > >  4 files changed, 45 insertions(+), 37 deletions(-)
> > > 
> > > Since we are splitting the original patch anyway, it's a bit strange
> > > to me that you are mixing arch/arm/mach-imx and drivers/usb/gadget
> > > in this patch.  I'm fine with it, since I assume all the patches to
> > > go via USB tree anyway.
> > > 
> > > > 
> > > > diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
> > > > index 6277baf..9bd5777 100644
> > > > --- a/arch/arm/mach-imx/devices/devices-common.h
> > > > +++ b/arch/arm/mach-imx/devices/devices-common.h
> > > > @@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
> > > >  
> > > >  #include <linux/fsl_devices.h>
> > > >  struct imx_fsl_usb2_udc_data {
> > > > +	const char *devid;
> > > >  	resource_size_t iobase;
> > > >  	resource_size_t irq;
> > > >  };
> > > > diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > > index 37e4439..fb527c7 100644
> > > > --- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > > +++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > > > @@ -11,35 +11,36 @@
> > > >  #include "../hardware.h"
> > > >  #include "devices-common.h"
> > > >  
> > > > -#define imx_fsl_usb2_udc_data_entry_single(soc)				\
> > > > +#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)			\
> > > >  	{								\
> > > > +		.devid = _devid,					\
> > > >  		.iobase = soc ## _USB_OTG_BASE_ADDR,			\
> > > >  		.irq = soc ## _INT_USB_OTG,				\
> > > >  	}
> > > >  
> > > >  #ifdef CONFIG_SOC_IMX25
> > > >  const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
> > > > -	imx_fsl_usb2_udc_data_entry_single(MX25);
> > > > +	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
> > > >  #endif /* ifdef CONFIG_SOC_IMX25 */
> > > >  
> > > >  #ifdef CONFIG_SOC_IMX27
> > > >  const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
> > > > -	imx_fsl_usb2_udc_data_entry_single(MX27);
> > > > +	imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
> > > >  #endif /* ifdef CONFIG_SOC_IMX27 */
> > > >  
> > > >  #ifdef CONFIG_SOC_IMX31
> > > >  const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
> > > > -	imx_fsl_usb2_udc_data_entry_single(MX31);
> > > > +	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
> > > >  #endif /* ifdef CONFIG_SOC_IMX31 */
> > > >  
> > > >  #ifdef CONFIG_SOC_IMX35
> > > >  const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
> > > > -	imx_fsl_usb2_udc_data_entry_single(MX35);
> > > > +	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
> > > >  #endif /* ifdef CONFIG_SOC_IMX35 */
> > > >  
> > > >  #ifdef CONFIG_SOC_IMX51
> > > >  const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
> > > > -	imx_fsl_usb2_udc_data_entry_single(MX51);
> > > > +	imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
> > > >  #endif
> > > >  
> > > >  struct platform_device *__init imx_add_fsl_usb2_udc(
> > > > @@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
> > > >  			.flags = IORESOURCE_IRQ,
> > > >  		},
> > > >  	};
> > > > -	return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
> > > > +	return imx_add_platform_device_dmamask(data->devid, -1,
> > > >  			res, ARRAY_SIZE(res),
> > > >  			pdata, sizeof(*pdata), DMA_BIT_MASK(32));
> > > >  }
> > > 
> > > <snip>
> > > 
> > > > +static const struct platform_device_id fsl_udc_devtype[] = {
> > > > +	{
> > > > +		.name = "imx-udc-mx25",
> > > > +	}, {
> > > > +		.name = "imx-udc-mx27",
> > > > +	}, {
> > > > +		.name = "imx-udc-mx31",
> > > > +	}, {
> > > > +		.name = "imx-udc-mx35",
> > > > +	}, {
> > > > +		.name = "imx-udc-mx51",
> > > > +	}
> > > > +};
> > > 
> > > From what I understand balbi's comment, he dislikes this full list of
> > > device id.  Instead, he prefers to something like below.
> > > 
> > > static const struct platform_device_id fsl_udc_devtype[] = {
> > > 	{
> > > 		.name = "imx-udc-mx27",
> > > 	}, {
> > > 		.name = "imx-udc-mx51",
> > > 	}
> > > };
> > > 
> > > It basically tells that we are handling two type of devices here, one
> > > is imx-udc-mx27 type and the other is imx-udc-mx51 type, with mx25/31/35
> > > completely compatible with mx27 type.  We choose mx27 instead of mx25
> > > to define the type because mx27 Si came out earlier than mx25.  That
> > > said, we generally choose the earlies SoC name to define a particular
> > > version of IP block, since hardware version is mostly unavailable or
> > > unreliable.
> > > 
> > > But that also means in platform code which create the platform_device,
> > > you will need to use name "imx-udc-mx27" for even mx25/31/35.
> > > 
> > > 	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
> > > 	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
> > > 	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
> > > 
> > > Considering this is a piece of code we will not use for any new
> > > hardware, I'm fine with either way.
> > > 
> > > So, balbi, it's all your call to accept the series as it is or ask for
> > > another iteration.
> 
> right :-)
> 
> > Thanks Shawn. Let's see Felipe's comment, nevertheless, I will send v6 patch
> > due to a compile error at mx25
> 
> Shawn is right.

In fact, this driver is just the temp solution, we will use chipidea
in future, so just take all i.mx usb as two kinds of ip are ok.

Do you want me to change like Shawn said, or current version is ok?

> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

^ permalink raw reply

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-17  9:32 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, Felipe Balbi, kernel, Shawn Guo,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130117092829.GA8251@nchen-desktop>

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

Hi,

On Thu, Jan 17, 2013 at 05:28:30PM +0800, Peter Chen wrote:

<trimming>

> > > > From what I understand balbi's comment, he dislikes this full list of
> > > > device id.  Instead, he prefers to something like below.
> > > > 
> > > > static const struct platform_device_id fsl_udc_devtype[] = {
> > > > 	{
> > > > 		.name = "imx-udc-mx27",
> > > > 	}, {
> > > > 		.name = "imx-udc-mx51",
> > > > 	}
> > > > };
> > > > 
> > > > It basically tells that we are handling two type of devices here, one
> > > > is imx-udc-mx27 type and the other is imx-udc-mx51 type, with mx25/31/35
> > > > completely compatible with mx27 type.  We choose mx27 instead of mx25
> > > > to define the type because mx27 Si came out earlier than mx25.  That
> > > > said, we generally choose the earlies SoC name to define a particular
> > > > version of IP block, since hardware version is mostly unavailable or
> > > > unreliable.
> > > > 
> > > > But that also means in platform code which create the platform_device,
> > > > you will need to use name "imx-udc-mx27" for even mx25/31/35.
> > > > 
> > > > 	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
> > > > 	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
> > > > 	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
> > > > 
> > > > Considering this is a piece of code we will not use for any new
> > > > hardware, I'm fine with either way.
> > > > 
> > > > So, balbi, it's all your call to accept the series as it is or ask for
> > > > another iteration.
> > 
> > right :-)
> > 
> > > Thanks Shawn. Let's see Felipe's comment, nevertheless, I will send v6 patch
> > > due to a compile error at mx25
> > 
> > Shawn is right.
> 
> In fact, this driver is just the temp solution, we will use chipidea
> in future, so just take all i.mx usb as two kinds of ip are ok.
> 
> Do you want me to change like Shawn said, or current version is ok?

yes, please resend with changes.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 1/6] usb: otg: Add an API to bind the USB controller and PHY
From: Roger Quadros @ 2013-01-17  9:37 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <1358348462-27693-2-git-send-email-kishon@ti.com>

On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
> New platforms are added which has multiple PHY's (of same type) and
> which has multiple USB controllers. The binding information has to be
> present in the PHY library (otg.c) in order for it to return the
> appropriate PHY whenever the USB controller request for the PHY. So
> added a new API to pass the binding information. This API should be
> called by platform specific initialization code.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/usb/otg/otg.c   |   37 +++++++++++++++++++++++++++++++++++++
>  include/linux/usb/phy.h |   22 ++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index a30c041..492ba2f 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -18,6 +18,7 @@
>  #include <linux/usb/otg.h>
>  
>  static LIST_HEAD(phy_list);
> +static LIST_HEAD(phy_bind_list);
>  static DEFINE_SPINLOCK(phy_lock);
>  
>  static struct usb_phy *__usb_find_phy(struct list_head *list,
> @@ -201,6 +202,42 @@ void usb_remove_phy(struct usb_phy *x)
>  }
>  EXPORT_SYMBOL(usb_remove_phy);
>  
> +/**
> + * usb_bind_phy - bind the phy and the controller that uses the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @index: index to specify the port number
> + * @phy_dev_name: the device name of the phy
> + *
> + * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
> + * be used when the phy driver registers the phy and when the controller
> + * requests this phy.
> + *
> + * To be used by platform specific initialization code.
> + */
> +struct usb_phy_bind __init *usb_bind_phy(const char *dev_name, u8 index,
> +				const char *phy_dev_name)
> +{
> +	struct usb_phy_bind *phy_bind;
> +	unsigned long flags;
> +
> +	phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
> +	if (!phy_bind) {
> +		pr_err("phy_bind(): No memory for phy_bind");

Function name in comment doesn't match the actual.
Instead, you could use
	pr_err("%s ...", __func__);

> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	phy_bind->dev_name = dev_name;
> +	phy_bind->phy_dev_name = phy_dev_name;
> +	phy_bind->index = index;
> +
> +	spin_lock_irqsave(&phy_lock, flags);
> +	list_add_tail(&phy_bind->list, &phy_bind_list);
> +	spin_unlock_irqrestore(&phy_lock, flags);
> +
> +	return phy_bind;
> +}
> +EXPORT_SYMBOL_GPL(usb_bind_phy);
> +
>  const char *otg_state_string(enum usb_otg_state state)
>  {
>  	switch (state) {
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index a29ae1e..fbeab1a 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -106,6 +106,21 @@ struct usb_phy {
>  			enum usb_device_speed speed);
>  };
>  
> +/**
> + * struct usb_phy_bind - represent the binding for the phy
> + * @dev_name: the device name of the device that will bind to the phy
> + * @phy_dev_name: the device name of the phy
> + * @index: used if a single controller uses multiple phys
> + * @phy: reference to the phy
> + * @list: to maintain a linked list of the binding information
> + */
> +struct usb_phy_bind {
> +	const char	*dev_name;
> +	const char	*phy_dev_name;
> +	u8		index;
> +	struct usb_phy	*phy;
> +	struct list_head list;
> +};
>  
>  /* for board-specific init logic */
>  extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
> @@ -151,6 +166,8 @@ extern struct usb_phy *devm_usb_get_phy(struct device *dev,
>  	enum usb_phy_type type);
>  extern void usb_put_phy(struct usb_phy *);
>  extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
> +extern struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8 index,
> +				const char *phy_dev_name);
>  #else
>  static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
>  {
> @@ -171,6 +188,11 @@ static inline void devm_usb_put_phy(struct device *dev, struct usb_phy *x)
>  {
>  }
>  
> +static inline struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8 index,
> +				const char *phy_dev_name)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  static inline int
> 

Controllers like ehci-omap which don't need OTG functionality would
benefit from this API. Can we make these PHY APIs not dependent on OTG /
OTG_UTILS?

cheers,
-roger

^ permalink raw reply

* Re: [RFC PATCH 2/6] ARM: OMAP: USB: Add phy binding information
From: Roger Quadros @ 2013-01-17  9:39 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <1358348462-27693-3-git-send-email-kishon@ti.com>

On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
> This is in preparation for the changes in PHY library to support adding
> and getting multiple PHYs of the same type. In the new design, the
> binding information between the PHY and the USB controller should be
> specified in the platform specific initialization code. So it's been
> done for OMAP platforms here.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> This kind-of binding should be done in all the platforms (I've done only
> for OMAP platform). 
>  arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 9d27e3f..bbe2fa5 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -24,6 +24,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/usb/musb.h>
> +#include <linux/usb/phy.h>
>  
>  #include "omap_device.h"
>  #include "soc.h"
> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>  	musb_plat.mode = board_data->mode;
>  	musb_plat.extvbus = board_data->extvbus;
>  
> -	if (cpu_is_omap44xx())
> +	if (cpu_is_omap44xx()) {
>  		musb_plat.has_mailbox = true;
> +		usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
> +	} else if (cpu_is_omap34xx()) {
> +		usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +	}

Are you sure than these OMAP platforms cannot be wired in any other way
to the PHY?

If they can be then this association must come from the board files or
device tree.

>  
>  	if (soc_is_am35xx()) {
>  		oh_name = "am35x_otg_hs";
> 

--
cheers,
-roger

^ permalink raw reply

* [PATCH v6 0/3] *** SUBJECT HERE ***
From: Peter Chen @ 2013-01-17 10:01 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

Changes for v6:
- Only using imx-udc-mx27 and imx-udc-mx51 stands for
all kinds of i.mx udc
- Fix below build error for platform_device_id at fsl_udc_core.c
drivers/usb/gadget/fsl_usb2_udc: struct platform_device_id is 24 bytes.? The last of 5 is:
| 0x69 0x6d 0x78 0x2d 0x75 0x64 0x63 0x2d 0x6d 0x78 0x35 0x31 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
+0x00 0x00 0x00
| FATAL: drivers/usb/gadget/fsl_usb2_udc: struct platform_device_id is not terminated with a NULL entry!
| make[1]: *** [__modpost] Error 1

Changes for v5:
- Using strcmp to get specific SoC
- Delete one cpu_is_mx35() as it has already pdata runtime check

Changes for v4:
- Using pdev's struct resource to do ioremap
- Add ioremap return value check

Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c                     |    6 +-
 arch/arm/mach-imx/clk-imx27.c                     |    6 +-
 arch/arm/mach-imx/clk-imx31.c                     |    6 +-
 arch/arm/mach-imx/clk-imx35.c                     |    6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c               |    6 +-
 arch/arm/mach-imx/devices/devices-common.h        |    1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++---
 drivers/usb/gadget/fsl_mxc_udc.c                  |   40 +++++++++++++-------
 drivers/usb/gadget/fsl_udc_core.c                 |   42 ++++++++++++--------
 drivers/usb/gadget/fsl_usb2_udc.h                 |    5 +-
 10 files changed, 78 insertions(+), 55 deletions(-)

^ permalink raw reply

* [PATCH v6 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-17 10:01 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416874-27011-1-git-send-email-peter.chen@freescale.com>

As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
already used pdata to differentiate runtime

Meanwhile we update the platform code accordingly.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 arch/arm/mach-imx/devices/devices-common.h        |    1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++----
 drivers/usb/gadget/fsl_mxc_udc.c                  |   24 +++++--------
 drivers/usb/gadget/fsl_udc_core.c                 |   38 ++++++++++++---------
 4 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
 
 #include <linux/fsl_devices.h>
 struct imx_fsl_usb2_udc_data {
+	const char *devid;
 	resource_size_t iobase;
 	resource_size_t irq;
 };
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..3c06bd9 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_fsl_usb2_udc_data_entry_single(soc)				\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)			\
 	{								\
+		.devid = _devid,					\
 		.iobase = soc ## _USB_OTG_BASE_ADDR,			\
 		.irq = soc ## _INT_USB_OTG,				\
 	}
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX25);
+	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX25 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX27);
+	imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX31);
+	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX35);
+	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 #ifdef CONFIG_SOC_IMX51
 const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX51);
+	imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
 #endif
 
 struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
 			.flags = IORESOURCE_IRQ,
 		},
 	};
-	return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
+	return imx_add_platform_device_dmamask(data->devid, -1,
 			res, ARRAY_SIZE(res),
 			pdata, sizeof(*pdata), DMA_BIT_MASK(32));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..de851e5 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
 	clk_prepare_enable(mxc_per_clk);
 
 	/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-	if (!cpu_is_mx51()) {
+	if (!strcmp(pdev->id_entry->name, "imx-udc-mx27")) {
 		freq = clk_get_rate(mxc_per_clk);
 		if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
 		    (freq < 59999000 || freq > 60001000)) {
@@ -82,17 +80,15 @@ eclkrate:
 void fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	if (cpu_is_mx35()) {
-		unsigned int v;
-
-		/* workaround ENGcm09152 for i.MX35 */
-		if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-			v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-			writel(v | USBPHYCTRL_EVDO,
-				MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-		}
+	unsigned int v;
+
+	/* workaround ENGcm09152 for i.MX35 */
+	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
+		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
+		writel(v | USBPHYCTRL_EVDO,
+			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
 	}
 
 	/* ULPI transceivers don't need usbpll */
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c19f7f1..5fc1d72 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -41,6 +41,7 @@
 #include <linux/fsl_devices.h>
 #include <linux/dmapool.h>
 #include <linux/delay.h>
+#include <linux/of_device.h>
 
 #include <asm/byteorder.h>
 #include <asm/io.h>
@@ -2438,11 +2439,6 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 	unsigned int i;
 	u32 dccparams;
 
-	if (strcmp(pdev->name, driver_name)) {
-		VDBG("Wrong device");
-		return -ENODEV;
-	}
-
 	udc_controller = kzalloc(sizeof(struct fsl_udc), GFP_KERNEL);
 	if (udc_controller == NULL) {
 		ERR("malloc udc failed\n");
@@ -2756,22 +2752,32 @@ static int fsl_udc_otg_resume(struct device *dev)
 
 	return fsl_udc_resume(NULL);
 }
-
 /*-------------------------------------------------------------------------
 	Register entry point for the peripheral controller driver
 --------------------------------------------------------------------------*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+	{
+		.name = "imx-udc-mx27",
+	}, {
+		.name = "imx-udc-mx51",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
 static struct platform_driver udc_driver = {
-	.remove  = __exit_p(fsl_udc_remove),
+	.remove		= __exit_p(fsl_udc_remove),
+	/* Just for FSL i.mx SoC currently */
+	.id_table	= fsl_udc_devtype,
 	/* these suspend and resume are not usb suspend and resume */
-	.suspend = fsl_udc_suspend,
-	.resume  = fsl_udc_resume,
-	.driver  = {
-		.name = (char *)driver_name,
-		.owner = THIS_MODULE,
-		/* udc suspend/resume called from OTG driver */
-		.suspend = fsl_udc_otg_suspend,
-		.resume  = fsl_udc_otg_resume,
+	.suspend	= fsl_udc_suspend,
+	.resume		= fsl_udc_resume,
+	.driver		= {
+			.name = (char *)driver_name,
+			.owner = THIS_MODULE,
+			/* udc suspend/resume called from OTG driver */
+			.suspend = fsl_udc_otg_suspend,
+			.resume  = fsl_udc_otg_resume,
 	},
 };
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v6 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
From: Peter Chen @ 2013-01-17 10:01 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416874-27011-1-git-send-email-peter.chen@freescale.com>

As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 arch/arm/mach-imx/clk-imx25.c       |    6 +++---
 arch/arm/mach-imx/clk-imx27.c       |    6 +++---
 arch/arm/mach-imx/clk-imx31.c       |    6 +++---
 arch/arm/mach-imx/clk-imx35.c       |    6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |    6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..2c570cd 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
 	clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
 	/* i.mx25 has the i.mx35 type cspi */
 	clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
 	clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
 	clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..16ccbd4 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx27");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
 	/* i.mx31 has the i.mx21 type uart */
 	clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..f0727e8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx27");
 	clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
 	clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil,
 	clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usboh3_per_gate], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usboh3_per_gate], "per", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ipg", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ahb", "imx-udc-mx51");
 	clk_register_clkdev(clk[nfc_gate], NULL, "imx51-nand");
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "imx-ssi.0");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "imx-ssi.1");
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v6 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-17 10:01 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416874-27011-1-git-send-email-peter.chen@freescale.com>

As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/gadget/fsl_mxc_udc.c  |   30 +++++++++++++++++++++++-------
 drivers/usb/gadget/fsl_udc_core.c |    4 +++-
 drivers/usb/gadget/fsl_usb2_udc.h |    5 +++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index de851e5..d3bd7b0 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET	0x608
+#define MX35_USBPHYCTRL_OFFSET		0x600
+#define USBPHYCTRL_OTGBASE_OFFSET	0x8
 #define USBPHYCTRL_EVDO			(1 << 23)
 
 int fsl_udc_clk_init(struct platform_device *pdev)
@@ -77,25 +78,40 @@ eclkrate:
 	return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	unsigned int v;
+	int ret = 0;
 
 	/* workaround ENGcm09152 for i.MX35 */
 	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+		unsigned int v;
+		struct resource *res = platform_get_resource
+			(pdev, IORESOURCE_MEM, 0);
+		void __iomem *phy_regs = ioremap(res->start +
+						MX35_USBPHYCTRL_OFFSET, 512);
+		if (!phy_regs) {
+			dev_err(&pdev->dev, "ioremap for phy address fails\n");
+			ret = -EINVAL;
+			goto ioremap_err;
+		}
+
+		v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
 		writel(v | USBPHYCTRL_EVDO,
-			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+			phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
+
+		iounmap(phy_regs);
 	}
 
+
+ioremap_err:
 	/* ULPI transceivers don't need usbpll */
 	if (pdata->phy_mode == FSL_USB2_PHY_ULPI) {
 		clk_disable_unprepare(mxc_per_clk);
 		mxc_per_clk = NULL;
 	}
+
+	return ret;
 }
 
 void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index 5fc1d72..667275c 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2543,7 +2543,9 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 		dr_controller_setup(udc_controller);
 	}
 
-	fsl_udc_clk_finalize(pdev);
+	ret = fsl_udc_clk_finalize(pdev);
+	if (ret)
+		goto err_free_irq;
 
 	/* Setup gadget structure */
 	udc_controller->gadget.ops = &fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..c6703bb 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -592,15 +592,16 @@ static inline struct ep_queue_head *get_qh_by_ep(struct fsl_ep *ep)
 struct platform_device;
 #ifdef CONFIG_ARCH_MXC
 int fsl_udc_clk_init(struct platform_device *pdev);
-void fsl_udc_clk_finalize(struct platform_device *pdev);
+int fsl_udc_clk_finalize(struct platform_device *pdev);
 void fsl_udc_clk_release(void);
 #else
 static inline int fsl_udc_clk_init(struct platform_device *pdev)
 {
 	return 0;
 }
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
+	return 0;
 }
 static inline void fsl_udc_clk_release(void)
 {
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v6 0/3] Fix the Build error for fsl_mxc_udc.c
From: Peter Chen @ 2013-01-17 10:03 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

Changes for v6:
- Only using imx-udc-mx27 and imx-udc-mx51 stands for
all kinds of i.mx udc
- Fix below build error for platform_device_id at fsl_udc_core.c
drivers/usb/gadget/fsl_usb2_udc: struct platform_device_id is 24 bytes.? The last of 5 is:
| 0x69 0x6d 0x78 0x2d 0x75 0x64 0x63 0x2d 0x6d 0x78 0x35 0x31 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
+0x00 0x00 0x00
| FATAL: drivers/usb/gadget/fsl_usb2_udc: struct platform_device_id is not terminated with a NULL entry!
| make[1]: *** [__modpost] Error 1

Changes for v5:
- Using strcmp to get specific SoC
- Delete one cpu_is_mx35() as it has already pdata runtime check

Changes for v4:
- Using pdev's struct resource to do ioremap
- Add ioremap return value check

Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c                     |    6 +-
 arch/arm/mach-imx/clk-imx27.c                     |    6 +-
 arch/arm/mach-imx/clk-imx31.c                     |    6 +-
 arch/arm/mach-imx/clk-imx35.c                     |    6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c               |    6 +-
 arch/arm/mach-imx/devices/devices-common.h        |    1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++---
 drivers/usb/gadget/fsl_mxc_udc.c                  |   40 +++++++++++++-------
 drivers/usb/gadget/fsl_udc_core.c                 |   42 ++++++++++++--------
 drivers/usb/gadget/fsl_usb2_udc.h                 |    5 +-
 10 files changed, 78 insertions(+), 55 deletions(-)

^ permalink raw reply

* [PATCH v6 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-17 10:03 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416997-29099-1-git-send-email-peter.chen@freescale.com>

As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/gadget/fsl_mxc_udc.c  |   30 +++++++++++++++++++++++-------
 drivers/usb/gadget/fsl_udc_core.c |    4 +++-
 drivers/usb/gadget/fsl_usb2_udc.h |    5 +++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index de851e5..d3bd7b0 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET	0x608
+#define MX35_USBPHYCTRL_OFFSET		0x600
+#define USBPHYCTRL_OTGBASE_OFFSET	0x8
 #define USBPHYCTRL_EVDO			(1 << 23)
 
 int fsl_udc_clk_init(struct platform_device *pdev)
@@ -77,25 +78,40 @@ eclkrate:
 	return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	unsigned int v;
+	int ret = 0;
 
 	/* workaround ENGcm09152 for i.MX35 */
 	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+		unsigned int v;
+		struct resource *res = platform_get_resource
+			(pdev, IORESOURCE_MEM, 0);
+		void __iomem *phy_regs = ioremap(res->start +
+						MX35_USBPHYCTRL_OFFSET, 512);
+		if (!phy_regs) {
+			dev_err(&pdev->dev, "ioremap for phy address fails\n");
+			ret = -EINVAL;
+			goto ioremap_err;
+		}
+
+		v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
 		writel(v | USBPHYCTRL_EVDO,
-			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+			phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
+
+		iounmap(phy_regs);
 	}
 
+
+ioremap_err:
 	/* ULPI transceivers don't need usbpll */
 	if (pdata->phy_mode == FSL_USB2_PHY_ULPI) {
 		clk_disable_unprepare(mxc_per_clk);
 		mxc_per_clk = NULL;
 	}
+
+	return ret;
 }
 
 void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index 5fc1d72..667275c 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2543,7 +2543,9 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 		dr_controller_setup(udc_controller);
 	}
 
-	fsl_udc_clk_finalize(pdev);
+	ret = fsl_udc_clk_finalize(pdev);
+	if (ret)
+		goto err_free_irq;
 
 	/* Setup gadget structure */
 	udc_controller->gadget.ops = &fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..c6703bb 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -592,15 +592,16 @@ static inline struct ep_queue_head *get_qh_by_ep(struct fsl_ep *ep)
 struct platform_device;
 #ifdef CONFIG_ARCH_MXC
 int fsl_udc_clk_init(struct platform_device *pdev);
-void fsl_udc_clk_finalize(struct platform_device *pdev);
+int fsl_udc_clk_finalize(struct platform_device *pdev);
 void fsl_udc_clk_release(void);
 #else
 static inline int fsl_udc_clk_init(struct platform_device *pdev)
 {
 	return 0;
 }
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
+	return 0;
 }
 static inline void fsl_udc_clk_release(void)
 {
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v6 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-17 10:03 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416997-29099-1-git-send-email-peter.chen@freescale.com>

As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
already used pdata to differentiate runtime

Meanwhile we update the platform code accordingly.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 arch/arm/mach-imx/devices/devices-common.h        |    1 +
 arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c |   15 ++++----
 drivers/usb/gadget/fsl_mxc_udc.c                  |   24 +++++--------
 drivers/usb/gadget/fsl_udc_core.c                 |   38 ++++++++++++---------
 4 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 6277baf..9bd5777 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
 
 #include <linux/fsl_devices.h>
 struct imx_fsl_usb2_udc_data {
+	const char *devid;
 	resource_size_t iobase;
 	resource_size_t irq;
 };
diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
index 37e4439..3c06bd9 100644
--- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
+++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
@@ -11,35 +11,36 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_fsl_usb2_udc_data_entry_single(soc)				\
+#define imx_fsl_usb2_udc_data_entry_single(soc, _devid)			\
 	{								\
+		.devid = _devid,					\
 		.iobase = soc ## _USB_OTG_BASE_ADDR,			\
 		.irq = soc ## _INT_USB_OTG,				\
 	}
 
 #ifdef CONFIG_SOC_IMX25
 const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX25);
+	imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX25 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX27);
+	imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX31);
+	imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX35);
+	imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 #ifdef CONFIG_SOC_IMX51
 const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
-	imx_fsl_usb2_udc_data_entry_single(MX51);
+	imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
 #endif
 
 struct platform_device *__init imx_add_fsl_usb2_udc(
@@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
 			.flags = IORESOURCE_IRQ,
 		},
 	};
-	return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
+	return imx_add_platform_device_dmamask(data->devid, -1,
 			res, ARRAY_SIZE(res),
 			pdata, sizeof(*pdata), DMA_BIT_MASK(32));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..de851e5 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
 	clk_prepare_enable(mxc_per_clk);
 
 	/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-	if (!cpu_is_mx51()) {
+	if (!strcmp(pdev->id_entry->name, "imx-udc-mx27")) {
 		freq = clk_get_rate(mxc_per_clk);
 		if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
 		    (freq < 59999000 || freq > 60001000)) {
@@ -82,17 +80,15 @@ eclkrate:
 void fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	if (cpu_is_mx35()) {
-		unsigned int v;
-
-		/* workaround ENGcm09152 for i.MX35 */
-		if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-			v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-			writel(v | USBPHYCTRL_EVDO,
-				MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-		}
+	unsigned int v;
+
+	/* workaround ENGcm09152 for i.MX35 */
+	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
+		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
+		writel(v | USBPHYCTRL_EVDO,
+			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
 	}
 
 	/* ULPI transceivers don't need usbpll */
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c19f7f1..5fc1d72 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -41,6 +41,7 @@
 #include <linux/fsl_devices.h>
 #include <linux/dmapool.h>
 #include <linux/delay.h>
+#include <linux/of_device.h>
 
 #include <asm/byteorder.h>
 #include <asm/io.h>
@@ -2438,11 +2439,6 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 	unsigned int i;
 	u32 dccparams;
 
-	if (strcmp(pdev->name, driver_name)) {
-		VDBG("Wrong device");
-		return -ENODEV;
-	}
-
 	udc_controller = kzalloc(sizeof(struct fsl_udc), GFP_KERNEL);
 	if (udc_controller == NULL) {
 		ERR("malloc udc failed\n");
@@ -2756,22 +2752,32 @@ static int fsl_udc_otg_resume(struct device *dev)
 
 	return fsl_udc_resume(NULL);
 }
-
 /*-------------------------------------------------------------------------
 	Register entry point for the peripheral controller driver
 --------------------------------------------------------------------------*/
-
+static const struct platform_device_id fsl_udc_devtype[] = {
+	{
+		.name = "imx-udc-mx27",
+	}, {
+		.name = "imx-udc-mx51",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
 static struct platform_driver udc_driver = {
-	.remove  = __exit_p(fsl_udc_remove),
+	.remove		= __exit_p(fsl_udc_remove),
+	/* Just for FSL i.mx SoC currently */
+	.id_table	= fsl_udc_devtype,
 	/* these suspend and resume are not usb suspend and resume */
-	.suspend = fsl_udc_suspend,
-	.resume  = fsl_udc_resume,
-	.driver  = {
-		.name = (char *)driver_name,
-		.owner = THIS_MODULE,
-		/* udc suspend/resume called from OTG driver */
-		.suspend = fsl_udc_otg_suspend,
-		.resume  = fsl_udc_otg_resume,
+	.suspend	= fsl_udc_suspend,
+	.resume		= fsl_udc_resume,
+	.driver		= {
+			.name = (char *)driver_name,
+			.owner = THIS_MODULE,
+			/* udc suspend/resume called from OTG driver */
+			.suspend = fsl_udc_otg_suspend,
+			.resume  = fsl_udc_otg_resume,
 	},
 };
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v6 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
From: Peter Chen @ 2013-01-17 10:03 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358416997-29099-1-git-send-email-peter.chen@freescale.com>

As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 arch/arm/mach-imx/clk-imx25.c       |    6 +++---
 arch/arm/mach-imx/clk-imx27.c       |    6 +++---
 arch/arm/mach-imx/clk-imx31.c       |    6 +++---
 arch/arm/mach-imx/clk-imx35.c       |    6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |    6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..2c570cd 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
 	clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
 	/* i.mx25 has the i.mx35 type cspi */
 	clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
 	clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
 	clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..16ccbd4 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx27");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
 	/* i.mx31 has the i.mx21 type uart */
 	clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..f0727e8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx27");
 	clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
 	clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil,
 	clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usboh3_per_gate], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usboh3_per_gate], "per", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ipg", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ahb", "imx-udc-mx51");
 	clk_register_clkdev(clk[nfc_gate], NULL, "imx51-nand");
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "imx-ssi.0");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "imx-ssi.1");
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH v6 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
From: Felipe Balbi @ 2013-01-17 10:58 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <1358416997-29099-4-git-send-email-peter.chen@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On Thu, Jan 17, 2013 at 06:03:17PM +0800, Peter Chen wrote:
> As we use platform_device_id for fsl-usb2-udc driver, it needs to
> change clk connection-id, or the related devm_clk_get will be failed.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>

do I get Acked-by for this one ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v6 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
From: Shawn Guo @ 2013-01-17 11:01 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: r58472, gregkh, linux-usb, Peter Chen, kernel, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20130117105833.GP10814@arwen.pp.htv.fi>

On Thu, Jan 17, 2013 at 12:58:33PM +0200, Felipe Balbi wrote:
> On Thu, Jan 17, 2013 at 06:03:17PM +0800, Peter Chen wrote:
> > As we use platform_device_id for fsl-usb2-udc driver, it needs to
> > change clk connection-id, or the related devm_clk_get will be failed.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> 
> do I get Acked-by for this one ?

Acked-by: Shawn Guo <shawn.guo@linaro.org>

^ permalink raw reply

* Re: [RFC PATCH 2/6] ARM: OMAP: USB: Add phy binding information
From: kishon @ 2013-01-17 11:03 UTC (permalink / raw)
  To: Roger Quadros
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <50F7C6C7.3030400@ti.com>

Hi,

On Thursday 17 January 2013 03:09 PM, Roger Quadros wrote:
> On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
>> This is in preparation for the changes in PHY library to support adding
>> and getting multiple PHYs of the same type. In the new design, the
>> binding information between the PHY and the USB controller should be
>> specified in the platform specific initialization code. So it's been
>> done for OMAP platforms here.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> This kind-of binding should be done in all the platforms (I've done only
>> for OMAP platform).
>>   arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> index 9d27e3f..bbe2fa5 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/io.h>
>>   #include <linux/usb/musb.h>
>> +#include <linux/usb/phy.h>
>>
>>   #include "omap_device.h"
>>   #include "soc.h"
>> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>>   	musb_plat.mode = board_data->mode;
>>   	musb_plat.extvbus = board_data->extvbus;
>>
>> -	if (cpu_is_omap44xx())
>> +	if (cpu_is_omap44xx()) {
>>   		musb_plat.has_mailbox = true;
>> +		usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
>> +	} else if (cpu_is_omap34xx()) {
>> +		usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +	}
>
> Are you sure than these OMAP platforms cannot be wired in any other way
> to the PHY?
>
> If they can be then this association must come from the board files or
> device tree.

I had given some thought about that and since all the existing OMAP 
platforms had it this way, I added it here.
You having pointed that out, I'll move these to board files.

Thanks
Kishon

^ permalink raw reply

* Re: [PATCH v3 1/2] memory-hotplug: introduce CONFIG_HAVE_BOOTMEM_INFO_NODE and revert register_page_bootmem_info_node() when platform not support
From: Lin Feng @ 2013-01-17 10:37 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-ia64, aquini, heiko.carstens, tangchen, dhowells,
	paul.gortmaker, paulus, jbeulich, hpa, sparclinux, linux-s390,
	x86, isimatu.yasuaki, mingo, gerald.schaefer, fenghua.yu,
	jiang.liu, wency, mel, tglx, kamezawa.hiroyu, tony.luck, laijs,
	linux-mm, gregkh, linux-kernel, minchan, schwidefsky, linux390,
	akpm, linuxppc-dev, davem
In-Reply-To: <20130116141436.GE343@dhcp22.suse.cz>

Hi Michal,

On 01/16/2013 10:14 PM, Michal Hocko wrote:
> On Wed 16-01-13 16:14:18, Lin Feng wrote:
> [...]
>> diff --git a/mm/Kconfig b/mm/Kconfig
>> index 278e3ab..f8c5799 100644
>> --- a/mm/Kconfig
>> +++ b/mm/Kconfig
>> @@ -162,10 +162,18 @@ config MOVABLE_NODE
>>  	  Say Y here if you want to hotplug a whole node.
>>  	  Say N here if you want kernel to use memory on all nodes evenly.
>>  
>> +#
>> +# Only be set on architectures that have completely implemented memory hotplug
>> +# feature. If you are not sure, don't touch it.
>> +#
>> +config HAVE_BOOTMEM_INFO_NODE
>> +	def_bool n
>> +
>>  # eventually, we can have this option just 'select SPARSEMEM'
>>  config MEMORY_HOTPLUG
>>  	bool "Allow for memory hot-add"
>>  	select MEMORY_ISOLATION
>> +	select HAVE_BOOTMEM_INFO_NODE if X86_64
>>  	depends on SPARSEMEM || X86_64_ACPI_NUMA
>>  	depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
>>  	depends on (IA64 || X86 || PPC_BOOK3S_64 || SUPERH || S390)
> 
> I am still not sure I understand the relation to MEMORY_HOTREMOVE.
> Is register_page_bootmem_info_node required/helpful even if
> !CONFIG_MEMORY_HOTREMOVE?
>From old kenrel's view register_page_bootmem_info_node() is defined in 
CONFIG_MEMORY_HOTPLUG_SPARSE, it registers some info for 
memory hotplug/remove. If we don't use MEMORY_HOTPLUG feature, this
function is empty, we don't need the info at all.
So this info is not required/helpful if !CONFIG_MEMORY_HOTREMOVE.
> 
> Also, now that I am thinking about that more, maybe it would
> be cleaner to put the select into arch/x86/Kconfig and do it
> same as ARCH_ENABLE_MEMORY_{HOTPLUG,HOTREMOVE} (and name it
> ARCH_HAVE_BOOTMEM_INFO_NODE).
> 
Maybe put it in mm/Kconfig is a better choice, because if one day someone implements
the register_page_bootmem_info_node() for other archs they will get some clues
here, that's it has been implemented on x86_64. 
But I'm not so sure...

thanks,
linfeng

^ permalink raw reply

* Re: [PATCH v6 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Fabio Estevam @ 2013-01-17 11:13 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <1358416874-27011-2-git-send-email-peter.chen@freescale.com>

On Thu, Jan 17, 2013 at 8:01 AM, Peter Chen <peter.chen@freescale.com> wrote:

>         /* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
> -       if (!cpu_is_mx51()) {
> +       if (!strcmp(pdev->id_entry->name, "imx-udc-mx27")) {]]

Shouldn't this be:
 if (!strcmp(pdev->id_entry->name, "imx-udc-mx51"))

^ permalink raw reply

* Re: [RFC PATCH 1/6] usb: otg: Add an API to bind the USB controller and PHY
From: kishon @ 2013-01-17 11:14 UTC (permalink / raw)
  To: Roger Quadros
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <50F7C643.600@ti.com>

Hi,

On Thursday 17 January 2013 03:07 PM, Roger Quadros wrote:
> On 01/16/2013 05:00 PM, Kishon Vijay Abraham I wrote:
>> New platforms are added which has multiple PHY's (of same type) and
>> which has multiple USB controllers. The binding information has to be
>> present in the PHY library (otg.c) in order for it to return the
>> appropriate PHY whenever the USB controller request for the PHY. So
>> added a new API to pass the binding information. This API should be
>> called by platform specific initialization code.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>   drivers/usb/otg/otg.c   |   37 +++++++++++++++++++++++++++++++++++++
>>   include/linux/usb/phy.h |   22 ++++++++++++++++++++++
>>   2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
>> index a30c041..492ba2f 100644
>> --- a/drivers/usb/otg/otg.c
>> +++ b/drivers/usb/otg/otg.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/usb/otg.h>
>>
>>   static LIST_HEAD(phy_list);
>> +static LIST_HEAD(phy_bind_list);
>>   static DEFINE_SPINLOCK(phy_lock);
>>
>>   static struct usb_phy *__usb_find_phy(struct list_head *list,
>> @@ -201,6 +202,42 @@ void usb_remove_phy(struct usb_phy *x)
>>   }
>>   EXPORT_SYMBOL(usb_remove_phy);
>>
>> +/**
>> + * usb_bind_phy - bind the phy and the controller that uses the phy
>> + * @dev_name: the device name of the device that will bind to the phy
>> + * @index: index to specify the port number
>> + * @phy_dev_name: the device name of the phy
>> + *
>> + * Fills the phy_bind structure with the dev_name and phy_dev_name. This will
>> + * be used when the phy driver registers the phy and when the controller
>> + * requests this phy.
>> + *
>> + * To be used by platform specific initialization code.
>> + */
>> +struct usb_phy_bind __init *usb_bind_phy(const char *dev_name, u8 index,
>> +				const char *phy_dev_name)
>> +{
>> +	struct usb_phy_bind *phy_bind;
>> +	unsigned long flags;
>> +
>> +	phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
>> +	if (!phy_bind) {
>> +		pr_err("phy_bind(): No memory for phy_bind");
>
> Function name in comment doesn't match the actual.
> Instead, you could use
> 	pr_err("%s ...", __func__);
>
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>> +	phy_bind->dev_name = dev_name;
>> +	phy_bind->phy_dev_name = phy_dev_name;
>> +	phy_bind->index = index;
>> +
>> +	spin_lock_irqsave(&phy_lock, flags);
>> +	list_add_tail(&phy_bind->list, &phy_bind_list);
>> +	spin_unlock_irqrestore(&phy_lock, flags);
>> +
>> +	return phy_bind;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_bind_phy);
>> +
>>   const char *otg_state_string(enum usb_otg_state state)
>>   {
>>   	switch (state) {
>> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
>> index a29ae1e..fbeab1a 100644
>> --- a/include/linux/usb/phy.h
>> +++ b/include/linux/usb/phy.h
>> @@ -106,6 +106,21 @@ struct usb_phy {
>>   			enum usb_device_speed speed);
>>   };
>>
>> +/**
>> + * struct usb_phy_bind - represent the binding for the phy
>> + * @dev_name: the device name of the device that will bind to the phy
>> + * @phy_dev_name: the device name of the phy
>> + * @index: used if a single controller uses multiple phys
>> + * @phy: reference to the phy
>> + * @list: to maintain a linked list of the binding information
>> + */
>> +struct usb_phy_bind {
>> +	const char	*dev_name;
>> +	const char	*phy_dev_name;
>> +	u8		index;
>> +	struct usb_phy	*phy;
>> +	struct list_head list;
>> +};
>>
>>   /* for board-specific init logic */
>>   extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
>> @@ -151,6 +166,8 @@ extern struct usb_phy *devm_usb_get_phy(struct device *dev,
>>   	enum usb_phy_type type);
>>   extern void usb_put_phy(struct usb_phy *);
>>   extern void devm_usb_put_phy(struct device *dev, struct usb_phy *x);
>> +extern struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8 index,
>> +				const char *phy_dev_name);
>>   #else
>>   static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
>>   {
>> @@ -171,6 +188,11 @@ static inline void devm_usb_put_phy(struct device *dev, struct usb_phy *x)
>>   {
>>   }
>>
>> +static inline struct usb_phy_bind *usb_bind_phy(const char *dev_name, u8 index,
>> +				const char *phy_dev_name)
>> +{
>> +	return NULL;
>> +}
>>   #endif
>>
>>   static inline int
>>
>
> Controllers like ehci-omap which don't need OTG functionality would
> benefit from this API. Can we make these PHY APIs not dependent on OTG /
> OTG_UTILS?

Actually much of whatever is in otg.c can be used by controllers which 
don't have OTG functionality (except otg_state_string). I vaguely 
remember, there was a patch that renamed otg.c to phy.c etc.. I'm not 
sure what happened to that.

Thanks
Kishon

^ permalink raw reply

* Re: [PATCH v6 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Lothar Waßmann @ 2013-01-17 11:25 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: r58472, gregkh, linux-usb, balbi, Peter Chen, kernel, shawn.guo,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <CAOMZO5Chu0bcXPqn+wWHwAqLu+27DvLx1=Bzaffpd-bA2ahaAQ@mail.gmail.com>

Hi,

Fabio Estevam writes:
> On Thu, Jan 17, 2013 at 8:01 AM, Peter Chen <peter.chen@freescale.com> wrote:
> 
> >         /* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
> > -       if (!cpu_is_mx51()) {
> > +       if (!strcmp(pdev->id_entry->name, "imx-udc-mx27")) {]]
> 
> Shouldn't this be:
>  if (!strcmp(pdev->id_entry->name, "imx-udc-mx51"))
>
The equivalent of !cpu_is_mx51() would be
strcmp(pdev->id_entry->name, "imx-udc-mx51") (without the '!') meaning
id_entry->name is different from "imx-udc-mx51".

I personally hate the '!strcmp()' notation and prefer 'strcmp() == 0'
though they are technically equivalent. Using '==' makes it much
clearer to the reader what is intended.
!strcmp() lets one assume that it is true when the comparison fails,
though actually the opposite is true.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

^ permalink raw reply

* Re: [RFC PATCH 2/6] ARM: OMAP: USB: Add phy binding information
From: Vivek Gautam @ 2013-01-17 12:11 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <1358348462-27693-3-git-send-email-kishon@ti.com>

Hi Kishon,


On Wed, Jan 16, 2013 at 8:30 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> This is in preparation for the changes in PHY library to support adding
> and getting multiple PHYs of the same type. In the new design, the
> binding information between the PHY and the USB controller should be
> specified in the platform specific initialization code. So it's been
> done for OMAP platforms here.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> This kind-of binding should be done in all the platforms (I've done only
> for OMAP platform).
>  arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
> index 9d27e3f..bbe2fa5 100644
> --- a/arch/arm/mach-omap2/usb-musb.c
> +++ b/arch/arm/mach-omap2/usb-musb.c
> @@ -24,6 +24,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/usb/musb.h>
> +#include <linux/usb/phy.h>
>
>  #include "omap_device.h"
>  #include "soc.h"
> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>         musb_plat.mode = board_data->mode;
>         musb_plat.extvbus = board_data->extvbus;
>
> -       if (cpu_is_omap44xx())
> +       if (cpu_is_omap44xx()) {
>                 musb_plat.has_mailbox = true;
> +               usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
> +       } else if (cpu_is_omap34xx()) {
> +               usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
> +       }

Just trying to understand things here.
Is the device name we are using here the platform device name (pdev->name) ?
If i am not wrong then for devices added through device tree
the device name would actually include the device address in the string,
wouldn't that make things somewhat weird, since we will have to add
such bindings
using this address string.

one more doubt please :)
is it possible to bind phy in the controllers' driver somehow ?

>
>         if (soc_is_am35xx()) {
>                 oh_name = "am35x_otg_hs";
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks & Regards
Vivek

^ permalink raw reply

* Re: [RFC PATCH 2/6] ARM: OMAP: USB: Add phy binding information
From: kishon @ 2013-01-17 12:31 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-doc, tony, linux, linux-sh, alexander.shishkin, stern,
	devicetree-discuss, linuxppc-dev, rob.herring, horms,
	haojian.zhuang, linux-omap, linux-arm-kernel, eric.y.miao,
	b-cousson, gregkh, linux-usb, linux-kernel, balbi, cbou, rob,
	dwmw2
In-Reply-To: <CAFp+6iGEC9KgfuQz6nqnAY5JrmtPG=HkGw5yjKw7-1=8AG_=dg@mail.gmail.com>

Hi,

On Thursday 17 January 2013 05:41 PM, Vivek Gautam wrote:
> Hi Kishon,
>
>
> On Wed, Jan 16, 2013 at 8:30 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> This is in preparation for the changes in PHY library to support adding
>> and getting multiple PHYs of the same type. In the new design, the
>> binding information between the PHY and the USB controller should be
>> specified in the platform specific initialization code. So it's been
>> done for OMAP platforms here.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> This kind-of binding should be done in all the platforms (I've done only
>> for OMAP platform).
>>   arch/arm/mach-omap2/usb-musb.c |    7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
>> index 9d27e3f..bbe2fa5 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -24,6 +24,7 @@
>>   #include <linux/dma-mapping.h>
>>   #include <linux/io.h>
>>   #include <linux/usb/musb.h>
>> +#include <linux/usb/phy.h>
>>
>>   #include "omap_device.h"
>>   #include "soc.h"
>> @@ -85,8 +86,12 @@ void __init usb_musb_init(struct omap_musb_board_data *musb_board_data)
>>          musb_plat.mode = board_data->mode;
>>          musb_plat.extvbus = board_data->extvbus;
>>
>> -       if (cpu_is_omap44xx())
>> +       if (cpu_is_omap44xx()) {
>>                  musb_plat.has_mailbox = true;
>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "omap-usb2.1.auto");
>> +       } else if (cpu_is_omap34xx()) {
>> +               usb_bind_phy("musb-hdrc.0.auto", 0, "twl4030_usb");
>> +       }
>
> Just trying to understand things here.
> Is the device name we are using here the platform device name (pdev->name) ?

it's pdev->dev.name.
> If i am not wrong then for devices added through device tree
> the device name would actually include the device address in the string,

Ideally for device tree, devm_usb_get_phy_by_phandle() introduced in the 
5th patch of this series should be used.
usb_bind_phy should be called from platform specific initialization 
files (like your board file), which won't get executed in dt boot.
> wouldn't that make things somewhat weird, since we will have to add
> such bindings
> using this address string.
>
> one more doubt please :)
> is it possible to bind phy in the controllers' driver somehow ?
Thats not going to be good. Why do you need to do that?

Thanks
Kishon

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox