RE: [RFC 2/8] ARM: twd: register clock event for 1 core SMP
Hi Rob,
On Mon, Feb 18, 2013 at 19:17:29, Rob Herring wrote:
On 02/18/2013 12:30 AM, Afzal Mohammed wrote:
Register percpu local timer for scheduler tick in the case of one core
SMP configuration. In other cases - secondary cpu's as well as boot
cpu's having more than one core, this is being registered as per
existing boot flow, with a difference that they happens after delay
calibration. Registering the clock for tick in case of one core should
be done before Kernel calibrates delay (this is required to boot,
unless local timer is the only one registered for tick). Registering
twd local timer at init_time (which platforms are doing now) helps
achieve that with the proposed change.
This helps in an almost booting Kernel (minimal) by only relying on
ARM parts for an A9 one core SMP.
arch/arm/kernel/smp_twd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 616268c..118f4f2 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -335,6 +335,9 @@ static int __init
twd_local_timer_common_register(struct device_node *np)
twd_get_clock(np);
+ if (num_possible_cpus() == 1)
+ twd_timer_setup(evt);
+
Shouldn't this be fixed in the core code, so the same issue is fixed for
all timers?
Is the following that you had in mind ? (with it, this patch as well as
1/8 would not be required),
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index d3a22be..2b6417d 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -42,6 +42,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs);
*/
extern void smp_init_cpus(void);
+extern void smp_monocore_timer_setup(void);
+
/*
* Provide a function to raise an IPI cross call on CPUs in callmap.
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 5f73f70..7431b77 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -504,6 +504,15 @@ int local_timer_register(struct local_timer_ops *ops)
}
#endif
+void __init smp_monocore_timer_setup(void)
+{
+ unsigned int cpu = smp_processor_id();
+ struct clock_event_device *evt = per_cpu(percpu_clockevent, cpu);
+
+ if (!lt_ops)
+ lt_ops-setup(evt);
+}
+
static void __cpuinit percpu_timer_setup(void)
{
unsigned int cpu = smp_processor_id();
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 955d92d..eb023ee 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -116,5 +116,6 @@ int __init register_persistent_clock(clock_access_fn
read_boot,
void __init time_init(void)
{
machine_desc-init_time();
+ smp_monocore_timer_setup();
sched_clock_postinit();
}
Regards
Afzal
Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
Hi,
On Tue, Feb 19, 2013 at 11:23:14AM +0530, Kishon Vijay Abraham I wrote:
The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. To obtain a reference to the PHY without
using phandle, the platform specfic intialization code (say from board file)
should have already called phy_bind with the binding information. The binding
information consists of phy's device name, phy user device name and an index.
The index is used when the same phy user binds to mulitple phys.
PHY drivers should create the PHY by passing phy_descriptor that has
describes the PHY (label, type etc..) and ops like init, exit, suspend,
resume,
s/has describes/describes
poweron, shutdown.
The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for the sysfs entry is added
in Documentation/ABI/testing/sysfs-class-phy.
you forgot to mention here that one case of re-use is OMAP5 where the
same PHY IP (different instances of it) is used for SATA and USB3
functionality, which means that we would have to maintain 2 drivers for
the same thing.
diff --git a/Documentation/ABI/testing/sysfs-class-phy
b/Documentation/ABI/testing/sysfs-class-phy
new file mode 100644
index 000..ffd9930
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-phy
@@ -0,0 +1,15 @@
+What:/sys/class/phy/phy/label
+Date:Feb 2013
+KernelVersion: 3.10
+Contact: Kishon Vijay Abraham I kis...@ti.com
+Description:
+ This is a read-only file for getting the label of the phy.
+
+What:/sys/class/phy/phy/bind
this will cause problems with the generic bind/unbind sysfs files which
are write-only. You should probably rename it to something else.
+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+the PHY. The PHY framework provides 4 APIs to get a reference to the PHY.
'it has to get a reference to it. This framework', decreases the
duplication of 'PHY'.
+struct phy *phy_get(struct device *dev, u8 index);
+struct phy *devm_phy_get(struct device *dev, u8 index);
+struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index);
+struct phy *devm_of_phy_get(struct device *dev, const char *phandle, u8
index);
+
+phy_get and devm_phy_get can be used to get the PHY in non-dt boot. This API
+uses the binding information added using the phy_bind API to find and return
+the appropriate PHY. The only difference between the two APIs is that
+devm_phy_get associates the device with the PHY using devres on successful
PHY
+get. On driver detach, release function is invoked on the the devres data and
s/the the/the/
+devres data is freed.
s/devres data/it/
+7. Current Status
+
+Currently only USB in OMAP is made to use this framework. However using the
+USB PHY library cannot be completely removed because it is intertwined with
+OTG. Once we move OTG out of PHY completely, using the old PHY library can be
+completely removed. SATA in OMAP will also more likely use this new framework
+and we should have a patch for it soon.
not sure if this should be in the documentation, but fair enough.
+static DEFINE_MUTEX(phy_list_mutex);
not sure if a mutex is enough to protect list traversal...
+struct phy_ops {
+ int (*init)(struct phy_descriptor *desc);
+ int (*exit)(struct phy_descriptor *desc);
+ int (*suspend)(struct phy_descriptor *desc);
+ int (*resume)(struct phy_descriptor *desc);
should you really have these two pointers ? I wonder if it wouldn't be
better to force runtime_pm down on the users and have phy_suspend() and
phy_resume() just be wrappers to pm_runtime_get() and pm_runtime_put().
Up to discussion, I guess.
+#if defined(CONFIG_GENERIC_PHY) || defined(CONFIG_GENERIC_PHY_MODULE)
#if IS_ENABLED(CONFIG_GENERIC_PHY)
+static inline struct phy *devm_phy_get(struct device *dev, u8 index)
+{
+ return ERR_PTR(-EINVAL);
would -EOPNOTSUPP fit better here (and all others)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 2/5] usb: phy: omap-usb2: use the new generic PHY framework
Hi,
On Tue, Feb 19, 2013 at 11:23:15AM +0530, Kishon Vijay Abraham I wrote:
Used the generic PHY framework API to create the PHY. omap_usb2_suspend
is split into omap_usb_suspend and omap_usb_resume in order to align
with the new framework.
However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/usb/phy/omap-usb2.c | 49
++
include/linux/usb/omap_usb.h |3 +++
2 files changed, 52 insertions(+)
diff --git a/drivers/usb/phy/omap-usb2.c b/drivers/usb/phy/omap-usb2.c
index 844ab68..924ae59 100644
--- a/drivers/usb/phy/omap-usb2.c
+++ b/drivers/usb/phy/omap-usb2.c
@@ -119,9 +119,48 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
return 0;
}
+static int omap_usb_suspend(struct phy_descriptor *desc)
+{
+ struct omap_usb *phy = desc_to_omapusb(desc);
+
+ if (!phy-is_suspended) {
I wonder if it wouldn't be better to have some refcounting done by phy
framework itself so that this only gets called when phy-refcount == 0
+ omap_control_usb_phy_power(phy-control_dev, 0);
coudln't you move this to -runtime_suspend() ?
+ pm_runtime_put_sync(phy-dev);
+ phy-is_suspended = 1;
+ }
+
+ return 0;
+}
+
+static int omap_usb_resume(struct phy_descriptor *desc)
+{
+ u32 ret;
+ struct omap_usb *phy = desc_to_omapusb(desc);
+
+ if (phy-is_suspended) {
+ ret = pm_runtime_get_sync(phy-dev);
+ if (ret 0) {
+ dev_err(phy-dev, get_sync failed with err %d\n,
+ ret);
+ return ret;
+ }
+ omap_control_usb_phy_power(phy-control_dev, 1);
likewise, this could go to -runtime_resume().
--
balbi
signature.asc
Description: Digital signature
RE: [PATCH, RFC 7/8] ARM: dts: am4372: initial support
Hi Felipe,
On Mon, Feb 18, 2013 at 23:52:40, Balbi, Felipe wrote:
On Mon, Feb 18, 2013 at 05:08:16PM +0530, Afzal Mohammed wrote:
+ uart1: serial@44e09000 {
+ compatible = ti,am4372-uart,ti,omap2-uart;
+ clock-frequency = 4800;
+ reg = 0x44e09000 0x2000;
+ interrupts = 0 72 0x4;
missing ti,hwmods ??
It is a deliberate omission. For minimal bootconsole booting, hwmod
help is not required. Hwmod details would be added gradually later,
current series doesn't register hwmod.
Regards
Afzal
Re: [PATCH, RFC 7/8] ARM: dts: am4372: initial support
On Tue, Feb 19, 2013 at 10:10:17AM +0100, Mohammed, Afzal wrote:
Hi Felipe,
On Mon, Feb 18, 2013 at 23:52:40, Balbi, Felipe wrote:
On Mon, Feb 18, 2013 at 05:08:16PM +0530, Afzal Mohammed wrote:
+ uart1: serial@44e09000 {
+ compatible = ti,am4372-uart,ti,omap2-uart;
+ clock-frequency = 4800;
+ reg = 0x44e09000 0x2000;
+ interrupts = 0 72 0x4;
missing ti,hwmods ??
It is a deliberate omission. For minimal bootconsole booting, hwmod
help is not required. Hwmod details would be added gradually later,
current series doesn't register hwmod.
understood, thanks
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH, RFC 0/8] ARM: AM43 (OMAP2+) boot support
On Monday 18 February 2013 05:05 PM, Afzal Mohammed wrote: (Resending, since it seems, LAKML doesn't accept patches with subject prefix only as RFC, but requires PATCH prefix also) Hi, This series adds minimal support to boot Linux on platforms having AM43 based SoC's. This is being sent as an RFC to seek opinion about modification in twd to register percpu local timer clock event for scheduler tick in the case of one core SMP. AM43 SoC's are based on ARM Cortex-A9. It is an ARM Cortex-A9 SMP configuration with one core (not uniprocessor configuration). AM43 is similar to AM335x in it's peripheral capabilities, with many of the peripheral register mapping's similar like that of uart. After looking at the specs, you don't need the SMP mode since ACP isn't being used. AM43 is in pre-silicon stage and currently there are no public documents. This series has been tested on a pre-silicon platform that emulates AM43 SoC, changes proposed here are minimal - to get it booting. Kernel was directly run without the help of bootloader - Images were directly loaded onto a pre-initialized RAM and ARM registers updated as required for booting. Changes have been made over linux-next (next-20130213) with three OF related reverts (which otherwise causes problem in other platforms also) and compiled with omap2plus_defconfig. Multiplatform option was enabled, while most of CONFIG options were deselected for a faster boot. Beagle bone boots as earlier with these changes. An interesting observation is that it may be possible to boot this platform to console without any platform specific modification to proper Kernel (by that I mean excluding DT sources) using Arnd's, [PATCH,RFC] default machine descriptor for multiplatform, along with a CLOCKSOURCE_OF_DECLARE for smp twd. TWD use for AM437x is also limited because these times stops in low power sates and there you will need broad-cast mechanism which again more of SMP machine feature. So I suggest to use the wakeup timer(GPT1) clock-event instead of local timer for the mentioned reason. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 2/8] ARM: twd: register clock event for 1 core SMP
On Monday 18 February 2013 05:07 PM, Afzal Mohammed wrote: Register percpu local timer for scheduler tick in the case of one core SMP configuration. In other cases - secondary cpu's as well as boot cpu's having more than one core, this is being registered as per existing boot flow, with a difference that they happens after delay calibration. Registering the clock for tick in case of one core should be done before Kernel calibrates delay (this is required to boot, unless local timer is the only one registered for tick). Registering twd local timer at init_time (which platforms are doing now) helps achieve that with the proposed change. This helps in an almost booting Kernel (minimal) by only relying on ARM parts for an A9 one core SMP. Signed-off-by: Afzal Mohammed af...@ti.com --- As mentioned in cover-letter, I don't think we have good reasoning to make TWD to work with UP configuration. Even you fix the timer code, there are more cascaded dependencies which is not worth the effort. Regards, Samtosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 3/8] ARM: twd: clock rate from DT (if no DT clk tree)
On Monday 18 February 2013 05:07 PM, Afzal Mohammed wrote: Add an optional property to find clock-frequency from DT. This helps as a fallback mechanism in case there is no representation of clock tree in DT. Signed-off-by: Afzal Mohammed af...@ti.com --- This won't work always because twd clock is CPU dependent and changes with CPU clock speed. And more importantly to get CPUFreq working, you need to provide the correct clock-node to TWD library. Refer OMAP4 clock data for reference. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 1/8] ARM: localtimer: return percpu clkevt on register
On Monday 18 February 2013 05:06 PM, Afzal Mohammed wrote: Return percpu clock event on local timer register. It is the boot cpu that calls this and it can use the returned percpu clock event to register a clock event in the case of SMP configuration with one core. SMP configuration with 1 core is UP :-) Jokes apart as said already, lets see whether we really need it. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] arm: omap2: mux: fix debugfs file permission
OMAP's debugfs interface creates one file
for each signal in the mux table, such file
provides a read method but didn't provide
read permission. Fix it.
Signed-off-by: Felipe Balbi ba...@ti.com
---
arch/arm/mach-omap2/mux.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 6a217c9..03fa4f4 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -739,8 +739,9 @@ static void __init omap_mux_dbg_create_entry(
list_for_each_entry(e, partition-muxmodes, node) {
struct omap_mux *m = e-mux;
- (void)debugfs_create_file(m-muxnames[0], S_IWUSR, mux_dbg_dir,
- m, omap_mux_dbg_signal_fops);
+ (void)debugfs_create_file(m-muxnames[0], S_IWUSR | S_IRUGO,
+ mux_dbg_dir, m,
+ omap_mux_dbg_signal_fops);
}
}
--
1.8.1.rc1.5.g7e0651a
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] arm: omap2: voltage: remove unnecessary header
nothing from linux/debugfs.h is used on voltage.c. Signed-off-by: Felipe Balbi ba...@ti.com --- arch/arm/mach-omap2/voltage.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c index 3ac8fe1..595bf1a 100644 --- a/arch/arm/mach-omap2/voltage.c +++ b/arch/arm/mach-omap2/voltage.c @@ -23,7 +23,6 @@ #include linux/io.h #include linux/err.h #include linux/export.h -#include linux/debugfs.h #include linux/slab.h #include linux/clk.h -- 1.8.1.rc1.5.g7e0651a -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 4/8] ARM: am33xx: ll debug config help
On Monday 18 February 2013 05:07 PM, Afzal Mohammed wrote: Selecting DEBUG_AM33XXUART1 routes low level debug messages to first UART instance of AM335x based SoC's. This selection is valid for upcoming AM43 based SoC's too. Make this information available upon configuring. Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/Kconfig.debug | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index aca..b717b78 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -542,6 +542,9 @@ choice config DEBUG_AM33XXUART1 bool AM33XX UART1 + help + Route low level debug messages to first uart instance + for boards based on am335 and am43 family of SoC's config DEBUG_ZOOM_UART bool Zoom2/3 UART With DT, IIRC DEBUGLL is broken. So did you hack debug-macro.S to get the earlyprintk working ? Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 5/8] ARM: OMAP2+: am43: Kconfig
On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: Add Kconfig option for AM43 family of SoC's, these are ARM Cortex A9 based (SMP configuration with 1 core). Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/mach-omap2/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 49ac3df..683fbaa 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -141,6 +141,17 @@ config SOC_AM33XX select MULTI_IRQ_HANDLER select COMMON_CLK +config SOC_AM43 + bool TI AM43 + depends on ARCH_OMAP2PLUS + default y + select CPU_V7 + select HAVE_SMP You don't need this + select LOCAL_TIMERS if SMP This one as well + select MULTI_IRQ_HANDLER And this one too I guess. Rest is fine. regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH, RFC 4/8] ARM: am33xx: ll debug config help
Hi Santosh,
On Tue, Feb 19, 2013 at 15:55:59, Shilimkar, Santosh wrote:
With DT, IIRC DEBUGLL is broken. So did you hack debug-macro.S
to get the earlyprintk working ?
No, on linux-next, ll debug works properly.
Regards
Afzal
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
Re: [PATCH, RFC 6/8] ARM: OMAP2+: am43: basic dt support
On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote:
Describe minimal DT boot machine details for AM43 based SoC's. AM43
family of SoC's are ARM Cortex-A9 based with one core in SMP
configuration. Low level debug could be achieved by selecting
DEBUG_AM33XXUART1. To boot AM43 SoC, this change is sufficient w.r.t
Kernel (considering the fact that strictly speaking DT sources does
not classify as a part of Kernel). Here private timer of the one and
only core is being used as clock event (as well as, as time source).
Signed-off-by: Afzal Mohammed af...@ti.com
---
As discussed already, lets just call this as Cortex-A9 single core
machine to avoid confusion.
arch/arm/mach-omap2/board-generic.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/mach-omap2/board-generic.c
b/arch/arm/mach-omap2/board-generic.c
index 0274ff7..e083f08 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -15,7 +15,10 @@
#include linux/of_irq.h
#include linux/of_platform.h
#include linux/irqdomain.h
+#include linux/irqchip.h
+#include asm/mach/map.h
+#include asm/smp_twd.h
#include asm/mach/arch.h
#include common.h
@@ -182,3 +185,18 @@ DT_MACHINE_START(OMAP5_DT, Generic OMAP5 (Flattened Device
Tree))
.restart= omap44xx_restart,
MACHINE_END
#endif
+
+#ifdef CONFIG_SOC_AM43
+static const char *am43_boards_compat[] __initdata = {
+ ti,am43,
+ NULL,
+};
+
+DT_MACHINE_START(AM43_DT, Generic AM43 (Flattened Device Tree))
+ .map_io = debug_ll_io_init,
+ .init_irq = irqchip_init,
With Arnds patch [1], the above can be dropped..
+ .init_machine = omap_generic_init,
+ .init_time = twd_local_timer_of_register,
This one needs to take care of other timers as well from start. So
better to take that approach from start.
Regards,
Santosh
[1] https://patchwork.kernel.org/patch/2074871/
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 8/8] ARM: dts: am43-pre-silicon support
On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: AM43 SoC is in pre-silicon stage, meanwhile it has been modelled in a pre-silicon platform. To validate and boot Linux in pre-silicon platform that emulates an AM43 SoC, add DT build support. As bootloader is not used, bootargs is passed through DT. Note: This would be replaced by an original board support. Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/boot/dts/Makefile | 3 ++- arch/arm/boot/dts/am43-pre-silicon.dts | 31 +++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/am43-pre-silicon.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 94d88b9..b434344 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -124,7 +124,8 @@ dtb-$(CONFIG_ARCH_OMAP2PLUS) += omap2420-h4.dtb \ omap5-evm.dtb \ am335x-evm.dtb \ am335x-evmsk.dtb \ - am335x-bone.dtb + am335x-bone.dtb \ + am43-pre-silicon.dtb dtb-$(CONFIG_ARCH_ORION5X) += orion5x-lacie-ethernet-disk-mini-v2.dtb dtb-$(CONFIG_ARCH_PRIMA2) += prima2-evb.dtb dtb-$(CONFIG_ARCH_U8500) += snowball.dtb \ diff --git a/arch/arm/boot/dts/am43-pre-silicon.dts b/arch/arm/boot/dts/am43-pre-silicon.dts new file mode 100644 index 000..b9c6297 --- /dev/null +++ b/arch/arm/boot/dts/am43-pre-silicon.dts Well the pre-silicon platform and the SOC are very close and at least the support you are adding here is exactly same. So lets just use am437x.dtb or something like that. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap2: twl-common: Add default power configuration
Hi Matthias,
On 02/15/2013 04:59 PM, Matthias Brugger wrote:
2013/2/1 Tony Lindgren t...@atomide.com:
Hi,
* Robert Nelson robertcnel...@gmail.com [130124 07:58]:
On Wed, Jan 23, 2013 at 12:50 PM, Matthias Brugger
matthias@gmail.com wrote:
This patch adds a generic power script configuration.
When rebooting an OMAP3530 at 125 MHz, the reboot hangs.
With the generic power script, TWL4030 will be reset
when a warm reset occures. This way the OMAP3530 does not
hang on reboot.
Both look OK to me. I've added Peter to cc, it's best that he queues
all the twl changes.
Peter any comments on this patch?
The patch looks good to me as well.
Are you maintaining the whole twl4030 support or just the codec driver?
Right now I'm maintaining the audio support (audio MFD, vibra, ASoC) in twl*
While I have done some cleanup in the twl-core and related drivers recently
and I'm reviewing patches sent for any *twl* driver (if I'm in the CC) I have
not declared myself as Maintainer of the twl stack.
The problem with the twl stack is that the drivers are spread around in
different subsystem so if one takes maintainer responsibility for the stack,
he/she need to have several entries in MAINTAINERS file to cover twl. I still
don't think it is a good idea to 'bloat' the MAINTAINERS file for this.
I'm happy to review patches. About a year ago we had internal discussion
regarding to twl in upstream and Tero Kristo 'volunteered' to review patches
as well.
I still think that the twl patches should be queued via the corresponding
subsystem (OMAP, MFD, Input, GPIO, PWM, etc).
--
Péter
Best regards,
Matthias
Regards,
Tony
Signed-off-by: Matthias Brugger matthias@gmail.com
---
arch/arm/mach-omap2/twl-common.c | 38
++
arch/arm/mach-omap2/twl-common.h | 1 +
2 files changed, 39 insertions(+)
diff --git a/arch/arm/mach-omap2/twl-common.c
b/arch/arm/mach-omap2/twl-common.c
index e49b40b..f096beb 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -120,6 +120,41 @@ static struct twl4030_audio_data omap3_audio_pdata = {
.codec = omap3_codec,
};
+static struct twl4030_ins wrst_seq[] __initdata = {
+ {MSG_SINGULAR(DEV_GRP_NULL, 0x1b, RES_STATE_OFF), 2},
+ {MSG_SINGULAR(DEV_GRP_P1, 0xf, RES_STATE_WRST), 15},
+ {MSG_SINGULAR(DEV_GRP_P1, 0x10, RES_STATE_WRST), 15},
+ {MSG_SINGULAR(DEV_GRP_P1, 0x7, RES_STATE_WRST), 0x60},
+ {MSG_SINGULAR(DEV_GRP_P1, 0x19, RES_STATE_ACTIVE), 2},
+ {MSG_SINGULAR(DEV_GRP_NULL, 0x1b, RES_STATE_ACTIVE), 2},
+};
+
+static struct twl4030_script wrst_script __initdata = {
+ .script = wrst_seq,
+ .size = ARRAY_SIZE(wrst_seq),
+ .flags = TWL4030_WRST_SCRIPT,
+};
+
+static struct twl4030_script *omap3_power_scripts[] __initdata = {
+ wrst_script,
+};
+
+static struct twl4030_resconfig omap3_rconfig[] = {
+ { .resource = RES_HFCLKOUT, .devgroup = DEV_GRP_P3, .type = -1,
+ .type2 = -1 },
+ { .resource = RES_VDD1, .devgroup = DEV_GRP_P1, .type = -1,
+ .type2 = -1 },
+ { .resource = RES_VDD2, .devgroup = DEV_GRP_P1, .type = -1,
+ .type2 = -1 },
+ { 0, 0},
+};
+
+static struct twl4030_power_data omap3_power_pdata = {
+ .scripts= omap3_power_scripts,
+ .num= ARRAY_SIZE(omap3_power_scripts),
+ .resource_config = omap3_rconfig,
+};
+
static struct regulator_consumer_supply omap3_vdda_dac_supplies[] = {
REGULATOR_SUPPLY(vdda_dac, omapdss_venc),
};
@@ -224,6 +259,9 @@ void __init omap3_pmic_get_config(struct
twl4030_platform_data *pmic_data,
if (pdata_flags TWL_COMMON_PDATA_AUDIO !pmic_data-audio)
pmic_data-audio = omap3_audio_pdata;
+ if (pdata_flags TWL_COMMON_PDATA_POWER !pmic_data-power)
+ pmic_data-power = omap3_power_pdata;
+
/* Common regulator configurations */
if (regulators_flags TWL_COMMON_REGULATOR_VDAC
!pmic_data-vdac)
pmic_data-vdac = omap3_vdac_idata;
diff --git a/arch/arm/mach-omap2/twl-common.h
b/arch/arm/mach-omap2/twl-common.h
index dcfbad5..dbeb905 100644
--- a/arch/arm/mach-omap2/twl-common.h
+++ b/arch/arm/mach-omap2/twl-common.h
@@ -7,6 +7,7 @@
#define TWL_COMMON_PDATA_BCI (1 1)
#define TWL_COMMON_PDATA_MADC (1 2)
#define TWL_COMMON_PDATA_AUDIO (1 3)
+#define TWL_COMMON_PDATA_POWER (1 4)
/* Common LDO regulators for TWL4030/TWL6030 */
#define TWL_COMMON_REGULATOR_VDAC (1 0)
--
1.7.11.7
Thanks for making this generic Matthias..
Tested-by: Robert Nelson robertcnel...@gmail.com
Tested on Beagle C4 (omap3530) and Beagle xM (DM3730, no regressions)
with this patch to enable it on the Beagle..
From 65004dafc8d37c69dd839803dc8ea5dcefd993df Mon Sep 17 00:00:00 2001
From: Robert Nelson
Re: [PATCH v2 0/5] Generic PHY Framework
On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. How does this relate to the generic PHY interfaces in drivers/net/phy? Do you expect that to get merged into drivers/phy in the long run, or do you want to keep the generic phy only for everything but ethernet? I think it would be problematic to have two alternative interfaces for ethernet PHYs because then an ethernet driver still needs to decide which subsystem to interface with. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH, RFC 8/8] ARM: dts: am43-pre-silicon support
Hi Santosh, On Tue, Feb 19, 2013 at 16:05:22, Shilimkar, Santosh wrote: On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: AM43 SoC is in pre-silicon stage, meanwhile it has been modelled in a pre-silicon platform. To validate and boot Linux in pre-silicon platform that emulates an AM43 SoC, add DT build support. Note: This would be replaced by an original board support. - am335x-bone.dtb + am335x-bone.dtb \ + am43-pre-silicon.dtb Well the pre-silicon platform and the SOC are very close and at least the support you are adding here is exactly same. So lets just use am437x.dtb or something like that. SoC support is already added in patch 7/8. This is board (which doesn't exist now) support, hence a pre-silicon temporary one to validate it. Regards Afzal
Re: [PATCH, RFC 5/8] ARM: OMAP2+: am43: Kconfig
On Tue, Feb 19, 2013 at 03:57:07PM +0530, Santosh Shilimkar wrote: On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: Add Kconfig option for AM43 family of SoC's, these are ARM Cortex A9 based (SMP configuration with 1 core). Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/mach-omap2/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 49ac3df..683fbaa 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -141,6 +141,17 @@ config SOC_AM33XX select MULTI_IRQ_HANDLER select COMMON_CLK +config SOC_AM43 +bool TI AM43 +depends on ARCH_OMAP2PLUS +default y +select CPU_V7 +select HAVE_SMP You don't need this actually, this is needed for CONFIG_SMP_ON_UP -- balbi signature.asc Description: Digital signature
Re: [PATCH, RFC 8/8] ARM: dts: am43-pre-silicon support
On Tuesday 19 February 2013 04:22 PM, Mohammed, Afzal wrote: Hi Santosh, On Tue, Feb 19, 2013 at 16:05:22, Shilimkar, Santosh wrote: On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: AM43 SoC is in pre-silicon stage, meanwhile it has been modelled in a pre-silicon platform. To validate and boot Linux in pre-silicon platform that emulates an AM43 SoC, add DT build support. Note: This would be replaced by an original board support. - am335x-bone.dtb + am335x-bone.dtb \ + am43-pre-silicon.dtb Well the pre-silicon platform and the SOC are very close and at least the support you are adding here is exactly same. So lets just use am437x.dtb or something like that. SoC support is already added in patch 7/8. This is board (which doesn't exist now) support, hence a pre-silicon temporary one to validate it. I mean we can call it am437x-xyxboard.dtb already considering the data here can be re-used. Boot-args can be used from default kernel config with CONFIG_CMDLINE_FORCE. No strong opinion if you still insist to have a pre-silicon dtb. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 5/8] ARM: OMAP2+: am43: Kconfig
On Tuesday 19 February 2013 04:26 PM, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 03:57:07PM +0530, Santosh Shilimkar wrote: On Monday 18 February 2013 05:08 PM, Afzal Mohammed wrote: Add Kconfig option for AM43 family of SoC's, these are ARM Cortex A9 based (SMP configuration with 1 core). Signed-off-by: Afzal Mohammed af...@ti.com --- arch/arm/mach-omap2/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 49ac3df..683fbaa 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -141,6 +141,17 @@ config SOC_AM33XX select MULTI_IRQ_HANDLER select COMMON_CLK +config SOC_AM43 + bool TI AM43 + depends on ARCH_OMAP2PLUS + default y + select CPU_V7 + select HAVE_SMP You don't need this actually, this is needed for CONFIG_SMP_ON_UP Ahh.. I missed that. Thanks Felipe for pointing it out. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, RFC 4/8] ARM: am33xx: ll debug config help
On Tuesday 19 February 2013 04:00 PM, Mohammed, Afzal wrote: Hi Santosh, On Tue, Feb 19, 2013 at 15:55:59, Shilimkar, Santosh wrote: With DT, IIRC DEBUGLL is broken. So did you hack debug-macro.S to get the earlyprintk working ? No, on linux-next, ll debug works properly. Indeed. Tony fixed that now. Some how I missed this patch on the list. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH, RFC 8/8] ARM: dts: am43-pre-silicon support
Hi Santosh, On Tue, Feb 19, 2013 at 16:30:13, Shilimkar, Santosh wrote: On Tuesday 19 February 2013 04:22 PM, Mohammed, Afzal wrote: SoC support is already added in patch 7/8. This is board (which doesn't exist now) support, hence a pre-silicon temporary one to validate it. I mean we can call it am437x-xyxboard.dtb already considering the data here can be re-used. Boot-args can be used from default kernel config with CONFIG_CMDLINE_FORCE. No strong opinion if you still insist to have a pre-silicon dtb. This patch would be replaced by original board, once it is known. This was included to make a working complete series and if someone wants (internally) to test the series as is. Regards Afzal
Re: [PATCH, RFC 8/8] ARM: dts: am43-pre-silicon support
On Tuesday 19 February 2013 04:33 PM, Mohammed, Afzal wrote: Hi Santosh, On Tue, Feb 19, 2013 at 16:30:13, Shilimkar, Santosh wrote: On Tuesday 19 February 2013 04:22 PM, Mohammed, Afzal wrote: SoC support is already added in patch 7/8. This is board (which doesn't exist now) support, hence a pre-silicon temporary one to validate it. I mean we can call it am437x-xyxboard.dtb already considering the data here can be re-used. Boot-args can be used from default kernel config with CONFIG_CMDLINE_FORCE. No strong opinion if you still insist to have a pre-silicon dtb. This patch would be replaced by original board, once it is known. This was included to make a working complete series and if someone wants (internally) to test the series as is. Ohh. I assumed it is for merge as well. Thanks for clarification. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH, RFC 0/8] ARM: AM43 (OMAP2+) boot support
Hi Santosh,
On Tue, Feb 19, 2013 at 15:39:32, Shilimkar, Santosh wrote:
After looking at the specs, you don't need the SMP mode since ACP
isn't being used.
TWD use for AM437x is also limited because these times stops in
low power sates and there you will need broad-cast mechanism which
again more of SMP machine feature.
So I suggest to use the wakeup timer(GPT1) clock-event instead
of local timer for the mentioned reason.
Ok, all your remaining comments are w.r.t SMP and usage of TWD
as clock event.
I am not that familiar with SMP things, upon starting on AM43,
observing the fact that it is an SMP with one core configuration
and that as per ARM MP Core TRM it can have local timer, I saw
it as a ray of hope to get Kernel booting early without hwmod,
and I jumped on that idea which I feel was natural. Without
spending too much time on it, though of taking feedback on this
approach, resulting this rfc.
Let me understand more on SMP, TWD, and explore possibility
of using GPT1 instead of TWD for clock event.
From the beginning I wondered on seeing that as per ARM TRM's
Cortex-A9 can be MP core with 1-4 core (instead of 2-4), while
another uniprocessor A9 configuration is present.
Regards
Afzal
N�r��yb�X��ǧv�^�){.n�+{��f��{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj��!�i
Re: [PATCH v2 0/5] Generic PHY Framework
Hi, On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote: On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. How does this relate to the generic PHY interfaces in drivers/net/phy? Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Do you expect that to get merged into drivers/phy in the long run, or do you want to keep the generic phy only for everything but ethernet? We'd like the PHY drivers spread all over the kernel to get merged to drivers/phy including Ethernet. Having said that, Ethernet itself has a huge PHY framework and it's going to be little challenging to adapt to the new PHY framework (of course there'll be changes in this PHY framework when we want to have network PHY added here). But adapting USB PHYs to the new framework will be simpler and will be taken first. Also when we add SATA PHY (OMAP5), it will make use of this framework as both SATA and USB3 uses the same PHY IP. I think it would be problematic to have two alternative interfaces for ethernet PHYs because then an ethernet driver still needs to decide which subsystem to interface with. Right. For now Ethernet should continue to use their own PHY abstraction layer till we are a bit more clear on how to move it to drivers/phy. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/1] OMAP4: DSS: Add panel for Blaze Tablet boards
On 2013-02-18 16:30, Ruslan Bilovol wrote: So, I'm still inclined to say that we shouldn't merge this. Thanks for explanation. Do you have some plan when DSS will be ready to port this driver? (3.10, 3.11 or later)? When it's ready =). The work depends also on common display framework, so it's not only up to me. That said, I have a prototype branch, with my version of CDF, that somewhat works for the couple of boards and panels that I have ported. It doesn't have DT code there, though (DSS DT is in a separate branch for now). If all goes extremely well, I could imagine 3.10 having the support allowing to port this driver. And if you're interested in some prototype code, kernel crashes, DSS debugging, etc, I can point you to my branch and give you some notes, so you could try porting this panel driver to (my version of) CDF. Give me a private email if you want to try that. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH, RFC 1/8] ARM: localtimer: return percpu clkevt on register
Hi,
On Mon, Feb 18, 2013 at 05:06:39PM +0530, Afzal Mohammed wrote:
@@ -315,6 +315,7 @@ static struct local_timer_ops twd_lt_ops __cpuinitdata = {
static int __init twd_local_timer_common_register(struct device_node *np)
{
int err;
+ struct clock_event_device *evt;
twd_evt = alloc_percpu(struct clock_event_device *);
if (!twd_evt) {
@@ -328,8 +329,8 @@ static int __init twd_local_timer_common_register(struct
device_node *np)
goto out_free;
}
- err = local_timer_register(twd_lt_ops);
- if (err)
+ evt = local_timer_register(twd_lt_ops);
+ if (IS_ERR(evt))
goto out_irq;
this is wrong. Will return 0 even in the error case. Correct version
would be:
if (IS_ERR(evt)) {
err = PTR_ERR(evt);
goto out_irq;
}
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH, RFC 2/8] ARM: twd: register clock event for 1 core SMP
On Tue, Feb 19, 2013 at 03:44:14PM +0530, Santosh Shilimkar wrote: On Monday 18 February 2013 05:07 PM, Afzal Mohammed wrote: Register percpu local timer for scheduler tick in the case of one core SMP configuration. In other cases - secondary cpu's as well as boot cpu's having more than one core, this is being registered as per existing boot flow, with a difference that they happens after delay calibration. Registering the clock for tick in case of one core should be done before Kernel calibrates delay (this is required to boot, unless local timer is the only one registered for tick). Registering twd local timer at init_time (which platforms are doing now) helps achieve that with the proposed change. This helps in an almost booting Kernel (minimal) by only relying on ARM parts for an A9 one core SMP. Signed-off-by: Afzal Mohammed af...@ti.com --- As mentioned in cover-letter, I don't think we have good reasoning to make TWD to work with UP configuration. Even you fix the timer code, there are more cascaded dependencies which is not worth the effort. if CONFIG_SMP_ON_UP is enabled, smp_twd.c can still be compiled, right ? -- balbi signature.asc Description: Digital signature
Re: [PATCH, RFC 2/8] ARM: twd: register clock event for 1 core SMP
On Tuesday 19 February 2013 05:44 PM, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 03:44:14PM +0530, Santosh Shilimkar wrote: On Monday 18 February 2013 05:07 PM, Afzal Mohammed wrote: Register percpu local timer for scheduler tick in the case of one core SMP configuration. In other cases - secondary cpu's as well as boot cpu's having more than one core, this is being registered as per existing boot flow, with a difference that they happens after delay calibration. Registering the clock for tick in case of one core should be done before Kernel calibrates delay (this is required to boot, unless local timer is the only one registered for tick). Registering twd local timer at init_time (which platforms are doing now) helps achieve that with the proposed change. This helps in an almost booting Kernel (minimal) by only relying on ARM parts for an A9 one core SMP. Signed-off-by: Afzal Mohammed af...@ti.com --- As mentioned in cover-letter, I don't think we have good reasoning to make TWD to work with UP configuration. Even you fix the timer code, there are more cascaded dependencies which is not worth the effort. if CONFIG_SMP_ON_UP is enabled, smp_twd.c can still be compiled, right ? Yep though just from deps pesrpective TWD is made available for ARM SMP machines as below config HAVE_ARM_TWD bool depends on SMP Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
On Tuesday 19 February 2013, kishon wrote: On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote: On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. How does this relate to the generic PHY interfaces in drivers/net/phy? Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the platform_device (or something else) that gets probed by its parent bus or the DT. The operations that you define for the PHY actually mirror some of the things that we have for a 'struct device', so I think it would be quite logical to do it the same way. Note that MDIO has both a 'bus' and a 'class', and what we want here is more like what the 'class' was meant for, except that for new classes, we should actually use a 'bus', since the long-term plan is to kill off the concept of a 'class'. I hope that was not too confusing. :) Do you expect that to get merged into drivers/phy in the long run, or do you want to keep the generic phy only for everything but ethernet? We'd like the PHY drivers spread all over the kernel to get merged to drivers/phy including Ethernet. Having said that, Ethernet itself has a huge PHY framework and it's going to be little challenging to adapt to the new PHY framework (of course there'll be changes in this PHY framework when we want to have network PHY added here). But adapting USB PHYs to the new framework will be simpler and will be taken first. Also when we add SATA PHY (OMAP5), it will make use of this framework as both SATA and USB3 uses the same PHY IP. I think you need to have at least a concept of where you want to end up. It's totally fine to introduce a new subsystem that does only the minimum of what you need here, and add things as you go, but if you already plan to merge the existing mdio bus into this, you should really have an idea of what that will mean in the long run. I think it would be problematic to have two alternative interfaces for ethernet PHYs because then an ethernet driver still needs to decide which subsystem to interface with. Right. For now Ethernet should continue to use their own PHY abstraction layer till we are a bit more clear on how to move it to drivers/phy. Ok. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/3] suspend/resume support for OMAP nand driver
This patch series adds low power transition support for OMAP NAND driver. With recent driver conversion of GPMC to platform_driver, pm_runtime calls can be used to handle module enable disable of GPMC. This is taken care patch #1. patch #2 is for GPMC suspend/resume support. This includes low power transition support of - GPMC module - ELM module User initiated suspend/resume sequence tested on am335x-evm with NAND flash support. Also tested in omap3-evm for user initiated suspend/resume sequence with NAND flash. This patch series based on [1] and is present in [2]. 1. ARM: OMAP2+: AM33XX: Add suspend-resume support http://comments.gmane.org/gmane.linux.ports.arm.omap/91405 2. https://github.com/avinashphilip/am335x_linux/commits/NAND_supend_resume_support Changes Since v2: - Remove calll back of nand_suspend from omap2 nand driver, as the same call already done from suspend activity mtd class driver. Philip Avinash (3): arm: gpmc: Converts GPMC driver to pm_runtime capable arm: gpmc: Low power transition support mtd: devices: elm: Low power transition support arch/arm/mach-omap2/gpmc.c | 30 +--- drivers/mtd/devices/elm.c | 54 2 files changed, 81 insertions(+), 3 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 3/4] mtd: devices: elm: Low power transition support
On Wed, Feb 13, 2013 at 18:13:03, Russell King - ARM Linux wrote:
On Wed, Feb 13, 2013 at 11:42:01AM +, Philip, Avinash wrote:
On Sat, Feb 09, 2013 at 15:52:44, Russell King - ARM Linux wrote:
On Thu, Feb 07, 2013 at 06:06:57PM +0530, Philip Avinash wrote:
+static int elm_suspend(struct device *dev)
+{
+ struct elm_info *info = dev_get_drvdata(dev);
+ wait_queue_head_t wq;
+ DECLARE_WAITQUEUE(wait, current);
+
+ init_waitqueue_head(wq);
+ while (1) {
+ /* Make sure that ELM not running */
+ if (info-idle) {
+ add_wait_queue(wq, wait);
+ schedule();
+ remove_wait_queue(wq, wait);
+ } else {
+ break;
+ }
+ }
The above code looks really wrong - it will just spin endlessly with the
waitqueues doing nothing useful. What are you trying to do here?
The intention of waitqeue is to make the suspend process really wait till
the ELM module finishes current activity. Although this type of protection
already achieved in mtd layer using nand_suspend(), this one is particularly
required for ELM module to really make sure that *any pending* corrections
to
finish really before gone to suspend.
I don't think you understand what's going on with the above, and why the
above is completely ineffective.
1. Your wait queue head is declared on stack, and there's no references
to it outside of this function. So _nothing_ can activate the wait
queue.
2. You're not changing the current thread's status away from TASK_RUNNING,
so schedule() will either return immediately or it will schedule another
task if it's time for this one to be preempted.
In other words, the above can be rewritten as:
while (info-idle)
schedule();
and it will still have the same effect.
Now, if you want to be kinder to the system, then you should use a
wait queue properly. Put the waitqueue head in struct elm_info. Use
wait_event() here. And call wake_up() where you set info-idle to
false.
I understood the issue. Thanks for the detailed explanation.
It seems the entire mechanism of ELM module state check is not required.
The ELM suspend procedure initiated only after the current MTD transaction
finishes and is ensured in MTD class driver.
So I can simply disable ELM module in suspend without any check.
Thanks
Avinash
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] arm: gpmc: Converts GPMC driver to pm_runtime capable
Support for pm_runtime add to GPMC driver.
Signed-off-by: Philip Avinash avinashphi...@ti.com
---
arch/arm/mach-omap2/gpmc.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 2c57f81..b1cd6c1 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -29,6 +29,7 @@
#include linux/of_mtd.h
#include linux/of_device.h
#include linux/mtd/nand.h
+#include linux/pm_runtime.h
#include linux/platform_data/mtd-nand-omap2.h
@@ -1317,7 +1318,8 @@ static int gpmc_probe(struct platform_device *pdev)
return PTR_ERR(gpmc_l3_clk);
}
- clk_prepare_enable(gpmc_l3_clk);
+ pm_runtime_enable(pdev-dev);
+ pm_runtime_get_sync(pdev-dev);
gpmc_dev = pdev-dev;
@@ -1329,7 +1331,7 @@ static int gpmc_probe(struct platform_device *pdev)
rc = gpmc_mem_init();
if (IS_ERR_VALUE(rc)) {
- clk_disable_unprepare(gpmc_l3_clk);
+ pm_runtime_put_sync(pdev-dev);
clk_put(gpmc_l3_clk);
dev_err(gpmc_dev, failed to reserve memory\n);
return rc;
@@ -1340,7 +1342,7 @@ static int gpmc_probe(struct platform_device *pdev)
rc = gpmc_probe_dt(pdev);
if (rc 0) {
- clk_disable_unprepare(gpmc_l3_clk);
+ pm_runtime_put_sync(pdev-dev);
clk_put(gpmc_l3_clk);
dev_err(gpmc_dev, failed to probe DT parameters\n);
return rc;
@@ -1353,6 +1355,8 @@ static __devexit int gpmc_remove(struct platform_device
*pdev)
{
gpmc_free_irq();
gpmc_mem_exit();
+ pm_runtime_put_sync(pdev-dev);
+ pm_runtime_disable(pdev-dev);
gpmc_dev = NULL;
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] arm: gpmc: Low power transition support
With GPMC converted to platform driver recently, adds low power
transition support in driver itself.
Signed-off-by: Philip Avinash avinashphi...@ti.com
---
Changes since v1:
Module disable enable added using pm_runtime support.
arch/arm/mach-omap2/gpmc.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index b1cd6c1..cc57988 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1361,9 +1361,29 @@ static __devexit int gpmc_remove(struct platform_device
*pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int gpmc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ omap3_gpmc_save_context();
+ pm_runtime_put_sync(pdev-dev);
+ return 0;
+}
+
+static int gpmc_resume(struct platform_device *pdev)
+{
+ pm_runtime_get_sync(pdev-dev);
+ omap3_gpmc_restore_context();
+ return 0;
+}
+#endif
+
static struct platform_driver gpmc_driver = {
.probe = gpmc_probe,
.remove = __devexit_p(gpmc_remove),
+#ifdef CONFIG_PM
+ .suspend= gpmc_suspend,
+ .resume = gpmc_resume,
+#endif
.driver = {
.name = DEVICE_NAME,
.owner = THIS_MODULE,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] mtd: devices: elm: Low power transition support
In low power modes of AM335X platforms, peripherals power is cut off.
This patch supports low power sleep transition support for ELM driver.
Signed-off-by: Philip Avinash avinashphi...@ti.com
---
Changes Since v2:
- Removes wait queue mechanism. The order of device creation ensures
that nand driver got suspended well before elm module suspend.
Hence no need to check elm module state on suspend.
drivers/mtd/devices/elm.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
index f034239..a41c968 100644
--- a/drivers/mtd/devices/elm.c
+++ b/drivers/mtd/devices/elm.c
@@ -20,6 +20,7 @@
#include linux/interrupt.h
#include linux/io.h
#include linux/of.h
+#include linux/sched.h
#include linux/pm_runtime.h
#include linux/platform_data/elm.h
@@ -379,6 +380,23 @@ static int elm_remove(struct platform_device *pdev)
return 0;
}
+static int elm_suspend(struct device *dev)
+{
+ pm_runtime_put_sync(dev);
+ return 0;
+}
+
+static int elm_resume(struct device *dev)
+{
+ struct elm_info *info = dev_get_drvdata(dev);
+
+ pm_runtime_get_sync(dev);
+ elm_config(dev, info-bch_type);
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(elm_pm_ops, elm_suspend, elm_resume);
+
#ifdef CONFIG_OF
static const struct of_device_id elm_of_match[] = {
{ .compatible = ti,am3352-elm },
@@ -392,6 +410,7 @@ static struct platform_driver elm_driver = {
.name = elm,
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(elm_of_match),
+ .pm = elm_pm_ops,
},
.probe = elm_probe,
.remove = elm_remove,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote:
+static struct class *phy_class;
+static LIST_HEAD(phy_list);
+static DEFINE_MUTEX(phy_list_mutex);
+static LIST_HEAD(phy_bind_list);
Hmm, so you actually do have a 'class'. There is a GregKH mandated ban on
new classes, meaning that one should be converted to a bus_type instead.
Also, you really should not need the global phy_list, phy_list_mutex
and phy_bind_list variables, since the driver core already provides
you with ways to iterate through devices on a class or bus.
+/**
+ * of_phy_get - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @phandle: name of the property holding the phy phandle value
+ * @index - the index of the phy
+ *
+ * Returns the phy associated with the given phandle value,
+ * after getting a refcount to it or -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded.
+ */
+struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index)
+{
+ struct phy *phy = NULL;
+ struct phy_bind *phy_map = NULL;
+ struct device_node *node;
+
+ if (!dev-of_node) {
+ dev_dbg(dev, device does not have a device node entry\n);
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev-of_node, phandle, index);
+ if (!node) {
+ dev_dbg(dev, failed to get %s phandle in %s node\n, phandle,
+ dev-of_node-full_name);
+ return ERR_PTR(-ENODEV);
+ }
I wonder whether this one should be of_parse_phandle_with_args() instead,
so you can have client-specific configuration in the property. Basically
instead of
phy = usbphy0 usbphy1;
you can pass an arbitrary number of arguments along with this, as
determined by some property in the phy node:
usbphy0: phy@1 {
#phy-cells = 1;
};
ehci@2 {
phy = usbphy0 23;
};
Which in turn leads to the argument (23) being passed into a phy_bind().
I also wonder if you should allow arbitrary names for the property.
Can't this always be called 'phy'? If you allow named phys, it would
more more consistent with other bindings to have a list of phy handles
in a property called phy, and a second property called phy-names
that contains the named strings.
+/**
+ * phy_create - create a new phy
+ * @dev: device that is creating the new phy
+ * @desc: descriptor of the phy
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
+{
+ int ret;
+ struct phy *phy;
+ struct phy_bind *phy_bind;
+ const char *devname = NULL;
...
+
+ devname = dev_name(dev);
+ device_initialize(phy-dev);
+ phy-desc = desc;
+ phy-dev.class = phy_class;
+ phy-dev.parent = dev;
+ phy-dev.bus = desc-bus;
+ ret = dev_set_name(phy-dev, %s, devname);
Passing a bus_type through the descriptor seems misplaced. What is this for?
Why is this function not just:
struct phy *phy_create(struct device *dev, const char *label, int type,
struct phy_ops *ops);
Passing a structure like you do here seems dangerous because when someone
decides to add members to the structure, existing code will not give a
build error but silently break.
+/**
+ * struct phy_ops - set of function pointers for performing phy operations
+ * @init: operation to be performed for initializing phy
+ * @exit: operation to be performed while exiting
+ * @suspend: suspending the phy
+ * @resume: resuming the phy
+ * @poweron: powering on the phy
+ * @shutdown: shutting down the phy
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops {
+ int (*init)(struct phy_descriptor *desc);
+ int (*exit)(struct phy_descriptor *desc);
+ int (*suspend)(struct phy_descriptor *desc);
+ int (*resume)(struct phy_descriptor *desc);
+ int (*poweron)(struct phy_descriptor *desc);
+ int (*shutdown)(struct phy_descriptor *desc);
+ struct module *owner;
+};
Shouldn't these take the 'struct phy' as an argument? struct phy_descriptor is
not even based on a 'struct device'.
+struct phy {
+ struct device dev;
+ struct phy_descriptor *desc;
+ struct list_headhead;
+};
Please kill off the 'head' member here and use the infrastructure we
already have. The concept of the phy_descriptor seems even more foreign
here, so best just collapse that into 'struct phy'. Maybe also rename
the structure to 'phy_device'.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP3: board-generic: Add missing omap3_init_late
The .init_late callback for OMAP3 has been missing for DT builds, which causes a lot of late PM initializations to be missed in turn. Signed-off-by: Rajendra Nayak rna...@ti.com --- arch/arm/mach-omap2/board-generic.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index 53cb380b..9b35329 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -103,6 +103,7 @@ DT_MACHINE_START(OMAP3_DT, Generic OMAP3 (Flattened Device Tree)) .init_irq = omap_intc_of_init, .handle_irq = omap3_intc_handle_irq, .init_machine = omap_generic_init, + .init_late = omap3_init_late, .timer = omap3_timer, .dt_compat = omap3_boards_compat, .restart= omap3xxx_restart, @@ -120,6 +121,7 @@ DT_MACHINE_START(OMAP3_GP_DT, Generic OMAP3-GP (Flattened Device Tree)) .init_irq = omap_intc_of_init, .handle_irq = omap3_intc_handle_irq, .init_machine = omap_generic_init, + .init_late = omap3_init_late, .timer = omap3_secure_timer, .dt_compat = omap3_gp_boards_compat, .restart= omap3xxx_restart, -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
Hi, On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: On Tuesday 19 February 2013, kishon wrote: On Tuesday 19 February 2013 04:14 PM, Arnd Bergmann wrote: On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote: Added a generic PHY framework that provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. This framework will be of use only to devices that uses external PHY (PHY functionality is not embedded within the controller). The intention of creating this framework is to bring the phy drivers spread all over the Linux kernel to drivers/phy to increase code re-use and to increase code maintainability. Comments to make PHY as bus wasn't done because PHY devices can be part of other bus and making a same device attached to multiple bus leads to bad design. How does this relate to the generic PHY interfaces in drivers/net/phy? Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a bit overkill IMO. Imagine an I2C-controlled PHY driver like isp1301, that driver will have to register i2c_driver and phy_driver, which looks weird to me. If the only substitute for class is a bus, we can't drop classes just yet, I'm afraid. Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make sense IMHO. platform_device (or something else) that gets probed by its parent bus or the DT. The operations that you define for the PHY actually mirror some of the things that we have for a 'struct device', so I think it would be quite logical to do it the same way. Note that MDIO has both a 'bus' and a 'class', and what we want here is more like what the 'class' was meant for, except that for new classes, we should actually use a 'bus', since the long-term plan is to kill off the concept of a 'class'. I hope that was not too confusing. :) :) -- balbi signature.asc Description: Digital signature
Re: [PATCH] ARM: OMAP3: board-generic: Add missing omap3_init_late
On Tuesday 19 February 2013 06:28 PM, Rajendra Nayak wrote: The .init_late callback for OMAP3 has been missing for DT builds, which causes a lot of late PM initializations to be missed in turn. Signed-off-by: Rajendra Nayak rna...@ti.com --- Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
Hi,
On Tuesday 19 February 2013 06:26 PM, Arnd Bergmann wrote:
On Tuesday 19 February 2013, Kishon Vijay Abraham I wrote:
+static struct class *phy_class;
+static LIST_HEAD(phy_list);
+static DEFINE_MUTEX(phy_list_mutex);
+static LIST_HEAD(phy_bind_list);
Hmm, so you actually do have a 'class'. There is a GregKH mandated ban on
new classes, meaning that one should be converted to a bus_type instead.
Also, you really should not need the global phy_list, phy_list_mutex
and phy_bind_list variables, since the driver core already provides
you with ways to iterate through devices on a class or bus.
ok.
+/**
+ * of_phy_get - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @phandle: name of the property holding the phy phandle value
+ * @index - the index of the phy
+ *
+ * Returns the phy associated with the given phandle value,
+ * after getting a refcount to it or -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded.
+ */
+struct phy *of_phy_get(struct device *dev, const char *phandle, u8 index)
+{
+ struct phy *phy = NULL;
+ struct phy_bind *phy_map = NULL;
+ struct device_node *node;
+
+ if (!dev-of_node) {
+ dev_dbg(dev, device does not have a device node entry\n);
+ return ERR_PTR(-EINVAL);
+ }
+
+ node = of_parse_phandle(dev-of_node, phandle, index);
+ if (!node) {
+ dev_dbg(dev, failed to get %s phandle in %s node\n, phandle,
+ dev-of_node-full_name);
+ return ERR_PTR(-ENODEV);
+ }
I wonder whether this one should be of_parse_phandle_with_args() instead,
so you can have client-specific configuration in the property. Basically
instead of
phy = usbphy0 usbphy1;
you can pass an arbitrary number of arguments along with this, as
determined by some property in the phy node:
usbphy0: phy@1 {
#phy-cells = 1;
};
ehci@2 {
phy = usbphy0 23;
};
Which in turn leads to the argument (23) being passed into a phy_bind().
I also wonder if you should allow arbitrary names for the property.
Can't this always be called 'phy'? If you allow named phys, it would
more more consistent with other bindings to have a list of phy handles
in a property called phy, and a second property called phy-names
that contains the named strings.
Ok. Makes sense. We should make both *phy* and *phy-cells* standard here.
+/**
+ * phy_create - create a new phy
+ * @dev: device that is creating the new phy
+ * @desc: descriptor of the phy
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, struct phy_descriptor *desc)
+{
+ int ret;
+ struct phy *phy;
+ struct phy_bind *phy_bind;
+ const char *devname = NULL;
...
+
+ devname = dev_name(dev);
+ device_initialize(phy-dev);
+ phy-desc = desc;
+ phy-dev.class = phy_class;
+ phy-dev.parent = dev;
+ phy-dev.bus = desc-bus;
+ ret = dev_set_name(phy-dev, %s, devname);
Passing a bus_type through the descriptor seems misplaced. What is this for?
I thought if we are adding ethernet phys here (say drivers/phy/net), we
can make phy_device_create() (currently in drivers/net/phy/phy_device.c)
call phy_create with bus_type set to mdio_bus_type. Then we can have all
the PHYs showing up in /sys/class/phy/ and ethernet can continue to use
its own phy abstraction layer.
Why is this function not just:
struct phy *phy_create(struct device *dev, const char *label, int type,
struct phy_ops *ops);
since while calling the callback functions using ops, there wont be
anyway to get back the device specific structure pointer.
struct phy_dev {
.
.
struct phy_descriptor desc;
void __iomem*base;
.
.
};
static int phy_resume(struct phy_descriptor *desc)
{
//if we dont pass a member of phy_dev while *phy_create* we can't get
back phy_dev from callback functions as used below.
struct phy_dev *phy = desc_to_omapusb(desc);
return 0;
}
static struct phy_ops ops = {
.resume = phy_resume,
.owner = THIS_MODULE,
};
Passing a structure like you do here seems dangerous because when someone
decides to add members to the structure, existing code will not give a
build error but silently break.
Not sure I understood this point. Care to explain?
+/**
+ * struct phy_ops - set of function pointers for performing phy operations
+ * @init: operation to be performed for initializing phy
+ * @exit: operation to be performed while exiting
+ * @suspend: suspending the phy
+ * @resume: resuming the phy
+ * @poweron: powering on the phy
+ * @shutdown: shutting down the phy
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops
Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
On Tuesday 19 February 2013, kishon wrote:
+
+ devname = dev_name(dev);
+ device_initialize(phy-dev);
+ phy-desc = desc;
+ phy-dev.class = phy_class;
+ phy-dev.parent = dev;
+ phy-dev.bus = desc-bus;
+ ret = dev_set_name(phy-dev, %s, devname);
Passing a bus_type through the descriptor seems misplaced. What is this for?
I thought if we are adding ethernet phys here (say drivers/phy/net), we
can make phy_device_create() (currently in drivers/net/phy/phy_device.c)
call phy_create with bus_type set to mdio_bus_type. Then we can have all
the PHYs showing up in /sys/class/phy/ and ethernet can continue to use
its own phy abstraction layer.
Hmm, that relies on the fact that mdio uses a 'bus_type' while the new phy
support uses a 'class', and it will break if we ever get to the point
where those two concepts are merged. I would rather not plan ahead here.
Why is this function not just:
struct phy *phy_create(struct device *dev, const char *label, int type,
struct phy_ops *ops);
since while calling the callback functions using ops, there wont be
anyway to get back the device specific structure pointer.
struct phy_dev {
.
.
struct phy_descriptor desc;
void __iomem*base;
.
.
};
static int phy_resume(struct phy_descriptor *desc)
{
//if we dont pass a member of phy_dev while *phy_create* we can't get
back phy_dev from callback functions as used below.
struct phy_dev *phy = desc_to_omapusb(desc);
return 0;
}
static struct phy_ops ops = {
.resume = phy_resume,
.owner = THIS_MODULE,
};
In other subsystems, that is what the device-private_data pointer is used
for, which you could also pass to phy_create, or set after calling that
function.
Passing a structure like you do here seems dangerous because when someone
decides to add members to the structure, existing code will not give a
build error but silently break.
Not sure I understood this point. Care to explain?
Nevermind, when I wrote that sentence, I had not yet noticed that the
phy_descriptor is kept around. I was thinking that the structure was
only used to pass more arguments into phy_create.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a bit overkill IMO. Imagine an I2C-controlled PHY driver like isp1301, that driver will have to register i2c_driver and phy_driver, which looks weird to me. If the only substitute for class is a bus, we can't drop classes just yet, I'm afraid. Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make sense IMHO. It's a fine line, but I think a phy is something that resembles a device more than an LED does. When I read patch 1, I also noticed and commented on the fact that it does use a 'class'. Now, according to Greg, we should use 'bus_type' instead of 'class' in new code. I originally disagreed with that concept, but he's the boss here and it's good if he has a vision how things should be lined out. In practice, there is little difference between a 'bus_type' and a 'class', so just replace any instance of the former with the latter in your head when reading the code ;-) I understand that there is not a real common bus here, and the bus_type infrastructure would basically be used as a way to represent each PHY in sysfs and provide a way to enumerate and look them up inside of the kernel. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
Hi, On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote: On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a bit overkill IMO. Imagine an I2C-controlled PHY driver like isp1301, that driver will have to register i2c_driver and phy_driver, which looks weird to me. If the only substitute for class is a bus, we can't drop classes just yet, I'm afraid. Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make sense IMHO. It's a fine line, but I think a phy is something that resembles a device more than an LED does. When I read patch 1, I also noticed and commented on the fact that it does use a 'class'. Now, according to Greg, we should use 'bus_type' instead of 'class' in new code. I originally disagreed with that concept, but he's the boss here and it's good if he has a vision how things should be lined out. In practice, there is little difference between a 'bus_type' and a 'class', so just replace any instance of the former with the latter in your head when reading the code ;-) it's not so simple :-) if we must use bus_type we need to introduce all the device/driver matching mechanism which isn't necessary with a class. I understand that there is not a real common bus here, and the bus_type infrastructure would basically be used as a way to represent each PHY in sysfs and provide a way to enumerate and look them up inside of the kernel. right, but maybe we need another mechanism. If, in the long run we must use bus_type, then eventually pwm, led, regulators, etc will all be converted to bus_type. It will look quite weird IMHO. Greg, can you pitch your suggestion here ? It would be great to hear your rationale behind dropping class infrastructure, couldn't find anything through Google and since feature-removal-schedule.txt has been removed (without adding it to feature-removal-schedule.txt, I must add :-) I don't know what's the idea behind removing classes. cheers -- balbi signature.asc Description: Digital signature
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [130214 12:51]: On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 But that's triggered by runtime PM from the device driver, right? Yes - for devices with drivers, it will hopefully be called by the driver. I think it's fine for hwmod to do that as requested by the device driver via runtime PM since hwmod is the bus glue making the drivers arch independent. I think the only piece we're missing for that is for driver to run something like pm_runtime_init() that initializes the address space to hwmod. In the current design, we might be able to do this during the driver's first call to pm_runtime_get*(). I think that's the first point that we can hook into the PM runtime code. Once the hwmod code is moved out to be a bus, I'm hoping we can do this through the driver making a dev-bus-enable_device() call - similar to the way PCI drivers call pci_enable_device(), but not bus-specific. That should remove our current dependency on CONFIG_PM_RUNTIME for OMAP devices to work correctly :-( Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. One thing that's unclear to me for the DT case is how we'll bind the driver-specific parts of the reset code to the device, particularly in the case where there's no driver present. It should be possible to place an initcall in the driver-specific header, but that seems like a hack. Any thoughts on this? 2. We need to be able to reset and idle some hardware even if the driver is not compiled in. Yep. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] arm: gpmc: Low power transition support
Philip Avinash avinashphi...@ti.com writes:
With GPMC converted to platform driver recently, adds low power
transition support in driver itself.
Signed-off-by: Philip Avinash avinashphi...@ti.com
---
Changes since v1:
Module disable enable added using pm_runtime support.
arch/arm/mach-omap2/gpmc.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index b1cd6c1..cc57988 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -1361,9 +1361,29 @@ static __devexit int gpmc_remove(struct
platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
This should be CONFIG_PM_SLEEP for suspend/resume specific stuff.
+static int gpmc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ omap3_gpmc_save_context();
+ pm_runtime_put_sync(pdev-dev);
+ return 0;
+}
+
+static int gpmc_resume(struct platform_device *pdev)
+{
+ pm_runtime_get_sync(pdev-dev);
+ omap3_gpmc_restore_context();
+ return 0;
+}
+#endif
+
static struct platform_driver gpmc_driver = {
.probe = gpmc_probe,
.remove = __devexit_p(gpmc_remove),
+#ifdef CONFIG_PM
+ .suspend= gpmc_suspend,
+ .resume = gpmc_resume,
+#endif
These are the legacy PM operations, you need to use the ones in the
drivers struct dev_pm_ops.
.driver = {
.name = DEVICE_NAME,
.owner = THIS_MODULE,
Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote: On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: It's a fine line, but I think a phy is something that resembles a device more than an LED does. When I read patch 1, I also noticed and commented on the fact that it does use a 'class'. Now, according to Greg, we should use 'bus_type' instead of 'class' in new code. I originally disagreed with that concept, but he's the boss here and it's good if he has a vision how things should be lined out. In practice, there is little difference between a 'bus_type' and a 'class', so just replace any instance of the former with the latter in your head when reading the code ;-) it's not so simple :-) if we must use bus_type we need to introduce all the device/driver matching mechanism which isn't necessary with a class. I think the idea is to use a bus_type that has devices but no drivers associated with it, but I might be misremembering things. I understand that there is not a real common bus here, and the bus_type infrastructure would basically be used as a way to represent each PHY in sysfs and provide a way to enumerate and look them up inside of the kernel. right, but maybe we need another mechanism. If, in the long run we must use bus_type, then eventually pwm, led, regulators, etc will all be converted to bus_type. It will look quite weird IMHO. Yes, it would be a bit unusual, I agree. Greg, can you pitch your suggestion here ? It would be great to hear your rationale behind dropping class infrastructure, couldn't find anything through Google and since feature-removal-schedule.txt has been removed (without adding it to feature-removal-schedule.txt, I must add :-) I don't know what's the idea behind removing classes. I believe for now, the idea is to not add any new classes, but keep the existing ones for compatibility. 'struct class_device' however was already removed and got turned into 'struct device'. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: The other option could be to allow custom ioremap function pointers based on address range in __arm_ioremap_pfn_caller() the same way as the SoC specific static mappings are allowed. That would require adding a function pointer to struct map_desc. Maybe that would do the trick? That sounds fine to me too. To me it makes sense to eventually handle the I/O mappings automatically from data in the DT -- hence the association with a bus device, which should be present in DT data. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
On Tue, Feb 19, 2013 at 03:30:01PM +, Paul Walmsley wrote: Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: The other option could be to allow custom ioremap function pointers based on address range in __arm_ioremap_pfn_caller() the same way as the SoC specific static mappings are allowed. That would require adding a function pointer to struct map_desc. Maybe that would do the trick? That sounds fine to me too. To me it makes sense to eventually handle the I/O mappings automatically from data in the DT -- hence the association with a bus device, which should be present in DT data. No. I really don't get why OMAP thinks it's soo special that it needs to go around adding lots and lots of new abstractions all over the place. Indirect ioremap() through a function pointer so you can overload it with OMAP specific crap? No way. Function pointers in map_desc - what the hell for? You must be totally mad if you think that's a way forward, I really don't see how you think this kind of crap would be remotely acceptable. Ok, so you have this problem that you need hwmod to touch a register which is also contained within the device resources as well. That's fine. You can do it one of two ways: 1. Call out from the driver at the appropriate points (which seems to be _after_ you've ioremapped it) to access the register. 2. Find the resource, temporarily map the register, access it, and then unmap it. No requirement what so ever to override ioremap(). AT ALL. So stop this madness now. As for a function pointer in struct map_desc. You're bonkers, the lot of you. map_desc is a structure describing the boot time static mappings which gets discarded. Nothing keeps any references around to it, and it is used at a time when *NOTHING ELSE* should be going on in the kernel other than building those static mappings. Not even any arch code poking about at devices. Or anything like that. Because if you think that's acceptable, then we'll need to flush the cache and TLB after each and every map_desc entry is touched - which brings a whole load of new pain and slowness to the kernel boot. So please, stop this idiotic madness now. If you want such things as pci_enable_device(), then what you're actually asking for is omap_enable_device() for OMAP devices. OMAP devices are already specific enough to OMAP SoCs (god knows, they have really complex and obscure behaviours that no one else in their right mind would want to copy) that calling out to omap specific functions would never really be a problem. No need for any of this crazy dev-bus shit either. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Generic PHY Framework
On Tue, Feb 19, 2013 at 03:28:17PM +, Arnd Bergmann wrote:
On Tuesday 19 February 2013, Felipe Balbi wrote:
On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote:
On Tuesday 19 February 2013, Felipe Balbi wrote:
On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote:
It's a fine line, but I think a phy is something that resembles a device
more than an LED does. When I read patch 1, I also noticed and commented
on the fact that it does use a 'class'. Now, according to Greg, we should
use 'bus_type' instead of 'class' in new code. I originally disagreed with
that concept, but he's the boss here and it's good if he has a vision
how things should be lined out.
In practice, there is little difference between a 'bus_type' and a
'class',
so just replace any instance of the former with the latter in your head
when reading the code ;-)
it's not so simple :-) if we must use bus_type we need to introduce all
the device/driver matching mechanism which isn't necessary with a class.
I think the idea is to use a bus_type that has devices but no drivers
associated with it, but I might be misremembering things.
but then drivers wouldn't probe ever, although devices would get
created, so maybe it'll work...
Greg, can you pitch your suggestion here ? It would be great to hear
your rationale behind dropping class infrastructure, couldn't find
anything through Google and since feature-removal-schedule.txt has been
removed (without adding it to feature-removal-schedule.txt, I must add
:-) I don't know what's the idea behind removing classes.
I believe for now, the idea is to not add any new classes, but keep
the existing ones for compatibility. 'struct class_device' however
was already removed and got turned into 'struct device'.
was there ever a struct class_device.
What about struct class ?
linux/device.h ::
334 struct class {
335 const char *name;
336 struct module *owner;
337
338 struct class_attribute *class_attrs;
339 struct device_attribute *dev_attrs;
340 struct bin_attribute*dev_bin_attrs;
341 struct kobject *dev_kobj;
342
343 int (*dev_uevent)(struct device *dev, struct kobj_uevent_env *env);
344 char *(*devnode)(struct device *dev, umode_t *mode);
345
346 void (*class_release)(struct class *class);
347 void (*dev_release)(struct device *dev);
348
349 int (*suspend)(struct device *dev, pm_message_t state);
350 int (*resume)(struct device *dev);
351
352 const struct kobj_ns_type_operations *ns_type;
353 const void *(*namespace)(struct device *dev);
354
355 const struct dev_pm_ops *pm;
356
357 struct subsys_private *p;
358 };
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 0/5] Generic PHY Framework
On 02/19/2013 04:05 PM, Felipe Balbi wrote: Hi, On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote: On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a bit overkill IMO. Imagine an I2C-controlled PHY driver like isp1301, that driver will have to register i2c_driver and phy_driver, which looks weird to me. If the only substitute for class is a bus, we can't drop classes just yet, I'm afraid. Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make sense IMHO. It's a fine line, but I think a phy is something that resembles a device more than an LED does. When I read patch 1, I also noticed and commented on the fact that it does use a 'class'. Now, according to Greg, we should use 'bus_type' instead of 'class' in new code. I originally disagreed with that concept, but he's the boss here and it's good if he has a vision how things should be lined out. In practice, there is little difference between a 'bus_type' and a 'class', so just replace any instance of the former with the latter in your head when reading the code ;-) it's not so simple :-) if we must use bus_type we need to introduce all the device/driver matching mechanism which isn't necessary with a class. You have the code for phy - device matching in your framework anyway. Using the bus infrastructure brings changes the open coded matching into bus callbacks. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 0/5] Generic PHY Framework
Hi, On Tue, Feb 19, 2013 at 05:07:09PM +0100, Marc Kleine-Budde wrote: On 02/19/2013 04:05 PM, Felipe Balbi wrote: Hi, On Tue, Feb 19, 2013 at 02:34:40PM +, Arnd Bergmann wrote: On Tuesday 19 February 2013, Felipe Balbi wrote: On Tue, Feb 19, 2013 at 12:33:54PM +, Arnd Bergmann wrote: Currently drivers/phy and drivers/net/phy are independent and are not related to each other. There are some fundamental differences on how these frameworks work. IIUC, the *net* uses bus layer (MDIO bus) to match a PHY device with a PHY driver and the Ethernet device uses the bus layer to get the PHY. The Generic PHY Framework however doesn't have any bus layer. The PHY should be like any other Platform Devices and Drivers and the framework will provide some APIs to register with the framework. And there are other APIs which any controller can use to get the PHY (for e.g., in the case of dt boot, it can use phandle to get a reference to the PHY). Hmm, I think the use of a bus_type for a PHY actually sounds quite appropriate. The actual PHY device would then be a child of the really ? I'm not so sure, the *bus* used by the PHY is ULPI, UTMI, UTMI+, PIP3, I2C, etc... adding another 'fake' bus representation is a bit overkill IMO. Imagine an I2C-controlled PHY driver like isp1301, that driver will have to register i2c_driver and phy_driver, which looks weird to me. If the only substitute for class is a bus, we can't drop classes just yet, I'm afraid. Imagine a regulator bus, a pwm bus, an LED bus etc. They don't make sense IMHO. It's a fine line, but I think a phy is something that resembles a device more than an LED does. When I read patch 1, I also noticed and commented on the fact that it does use a 'class'. Now, according to Greg, we should use 'bus_type' instead of 'class' in new code. I originally disagreed with that concept, but he's the boss here and it's good if he has a vision how things should be lined out. In practice, there is little difference between a 'bus_type' and a 'class', so just replace any instance of the former with the latter in your head when reading the code ;-) it's not so simple :-) if we must use bus_type we need to introduce all the device/driver matching mechanism which isn't necessary with a class. You have the code for phy - device matching in your framework anyway. Using the bus infrastructure brings changes the open coded matching into bus callbacks. it's not the same thing. Current matching is just to figure out which phy belongs to which user. The bus matching rules are to bind a device with its driver, but that part has been taken care of by the underlying control bus used by the phy, be it i2c, spi, or whatever else. -- balbi signature.asc Description: Digital signature
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
* Russell King - ARM Linux li...@arm.linux.org.uk [130219 07:49]: On Tue, Feb 19, 2013 at 03:30:01PM +, Paul Walmsley wrote: Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: The other option could be to allow custom ioremap function pointers based on address range in __arm_ioremap_pfn_caller() the same way as the SoC specific static mappings are allowed. That would require adding a function pointer to struct map_desc. Maybe that would do the trick? That sounds fine to me too. To me it makes sense to eventually handle the I/O mappings automatically from data in the DT -- hence the association with a bus device, which should be present in DT data. No. I really don't get why OMAP thinks it's soo special that it needs to go around adding lots and lots of new abstractions all over the place. Hey it's actually quite the opposite. We're trying to find a generic way of doing things in order to avoid omap specific driver hacks. Indirect ioremap() through a function pointer so you can overload it with OMAP specific crap? No way. Function pointers in map_desc - what the hell for? You must be totally mad if you think that's a way forward, I really don't see how you think this kind of crap would be remotely acceptable. Ok, so you have this problem that you need hwmod to touch a register which is also contained within the device resources as well. That's fine. You can do it one of two ways: 1. Call out from the driver at the appropriate points (which seems to be _after_ you've ioremapped it) to access the register. 2. Find the resource, temporarily map the register, access it, and then unmap it. No requirement what so ever to override ioremap(). AT ALL. So stop this madness now. As for a function pointer in struct map_desc. You're bonkers, the lot of you. map_desc is a structure describing the boot time static mappings which gets discarded. Nothing keeps any references around to it, and it is used at a time when *NOTHING ELSE* should be going on in the kernel other than building those static mappings. Not even any arch code poking about at devices. Or anything like that. Because if you think that's acceptable, then we'll need to flush the cache and TLB after each and every map_desc entry is touched - which brings a whole load of new pain and slowness to the kernel boot. So please, stop this idiotic madness now. OK good point. Also it would be risky for things going wrong trying to try set up more complex stuff that early as then debugging it again depends on DEBUG_LL. If you want such things as pci_enable_device(), then what you're actually asking for is omap_enable_device() for OMAP devices. OMAP devices are already specific enough to OMAP SoCs (god knows, they have really complex and obscure behaviours that no one else in their right mind would want to copy) that calling out to omap specific functions would never really be a problem. I'd rather avoid adding omap_enable_device() calls to drivers as we really want to keep the drivers generic. But maybe there could be some generic bus_enable_device() function pointer that could be populated by the bus code during init. No need for any of this crazy dev-bus shit either. Paul's suggestion on configuring things with pm_runtime_get() seems doable and is generic. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
* Paul Walmsley p...@pwsan.com [130219 07:30]: Hi, On Thu, 14 Feb 2013, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [130214 12:51]: On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 But that's triggered by runtime PM from the device driver, right? Yes - for devices with drivers, it will hopefully be called by the driver. I think it's fine for hwmod to do that as requested by the device driver via runtime PM since hwmod is the bus glue making the drivers arch independent. I think the only piece we're missing for that is for driver to run something like pm_runtime_init() that initializes the address space to hwmod. In the current design, we might be able to do this during the driver's first call to pm_runtime_get*(). I think that's the first point that we can hook into the PM runtime code. Sounds doable and generic for now. Once the hwmod code is moved out to be a bus, I'm hoping we can do this through the driver making a dev-bus-enable_device() call - similar to the way PCI drivers call pci_enable_device(), but not bus-specific. That should remove our current dependency on CONFIG_PM_RUNTIME for OMAP devices to work correctly :-( Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. One thing that's unclear to me for the DT case is how we'll bind the driver-specific parts of the reset code to the device, particularly in the case where there's no driver present. It should be possible to place an initcall in the driver-specific header, but that seems like a hack. Any thoughts on this? I think we can just initialize the driver-specific reset functions in hwmod code and those can just call the driver specific inline functions as needed in the driver specific header files. 2. We need to be able to reset and idle some hardware even if the driver is not compiled in. Yep. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Tue, Feb 19, 2013 at 08:38:20AM -0800, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [130214 12:51]: On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 But that's triggered by runtime PM from the device driver, right? Yes - for devices with drivers, it will hopefully be called by the driver. I think it's fine for hwmod to do that as requested by the device driver via runtime PM since hwmod is the bus glue making the drivers arch independent. I think the only piece we're missing for that is for driver to run something like pm_runtime_init() that initializes the address space to hwmod. In the current design, we might be able to do this during the driver's first call to pm_runtime_get*(). I think that's the first point that we can hook into the PM runtime code. Sounds doable and generic for now. this isn't really a nice way IMHO. You will end up with drivers assuming that certain details will be initialized whenever it calls pm_runtime_get() for the first time. Then, for devices such as UART which, when used as console, will not get pm_domain-runtime_resume() called because pm_runtime_set_active() has been called right before. IOW, this will not work for all cases and very soon we will have bug reports. Once the hwmod code is moved out to be a bus, I'm hoping we can do this through the driver making a dev-bus-enable_device() call - similar to the way PCI drivers call pci_enable_device(), but not bus-specific. That should remove our current dependency on CONFIG_PM_RUNTIME for OMAP devices to work correctly :-( Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. this wouldn't work. How would you write a reset for i2c which can be called for all instances even when there is no driver loaded ? What would be the arguments to such a call ? If you have something like: static inline void omap_i2c_reset(int bus_nr); you would need to keep track of the bus numbers even when there is no driver around and if there is a driver, you would have to ask i2c-omap.c to track all available instances and convert that number to the proper struct omap_i2c pointer. None of those sound like a good plan IMO. From a driver perspective, we want all our calls to have our private structure or a *dev as argument. Those are only avaiable when driver is around and anything which doesn't take those as argument will force driver to add extra unnecessary churn. One thing that's unclear to me for the DT case is how we'll bind the driver-specific parts of the reset code to the device, particularly in the case where there's no driver present. It should be possible to place an initcall in the driver-specific header, but that seems like a hack. Any thoughts on this? I think we can just initialize the driver-specific reset functions in hwmod code and those can just call the driver specific inline functions as needed in the driver specific header files. please don't. This will not work as you expect, we will just add more special cases to drivers which will be even more complex to maintain. What you want is a generic reset callback somewhere. For cases where there is no driver around, then I'm not sure what to do, but doing early reset sounds like a better plan than late reset, so resetting on omap_device_build_from_dt() sounds better IMHO, although it concerns me cases of -EPROBE_DEFER and what that would mean in the long run... -- balbi signature.asc Description: Digital signature
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
* Felipe Balbi ba...@ti.com [130219 09:01]: Hi, On Tue, Feb 19, 2013 at 08:38:20AM -0800, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [130214 12:51]: On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 But that's triggered by runtime PM from the device driver, right? Yes - for devices with drivers, it will hopefully be called by the driver. I think it's fine for hwmod to do that as requested by the device driver via runtime PM since hwmod is the bus glue making the drivers arch independent. I think the only piece we're missing for that is for driver to run something like pm_runtime_init() that initializes the address space to hwmod. In the current design, we might be able to do this during the driver's first call to pm_runtime_get*(). I think that's the first point that we can hook into the PM runtime code. Sounds doable and generic for now. this isn't really a nice way IMHO. You will end up with drivers assuming that certain details will be initialized whenever it calls pm_runtime_get() for the first time. Then, for devices such as UART which, when used as console, will not get pm_domain-runtime_resume() called because pm_runtime_set_active() has been called right before. IOW, this will not work for all cases and very soon we will have bug reports. Some generic way to init this from drivers is needed, I'm out of ideas here though. Once the hwmod code is moved out to be a bus, I'm hoping we can do this through the driver making a dev-bus-enable_device() call - similar to the way PCI drivers call pci_enable_device(), but not bus-specific. That should remove our current dependency on CONFIG_PM_RUNTIME for OMAP devices to work correctly :-( Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. this wouldn't work. How would you write a reset for i2c which can be called for all instances even when there is no driver loaded ? What would be the arguments to such a call ? If you have something like: static inline void omap_i2c_reset(int bus_nr); you would need to keep track of the bus numbers even when there is no driver around and if there is a driver, you would have to ask i2c-omap.c to track all available instances and convert that number to the proper struct omap_i2c pointer. None of those sound like a good plan IMO. From a driver perspective, we want all our calls to have our private structure or a *dev as argument. Those are only avaiable when driver is around and anything which doesn't take those as argument will force driver to add extra unnecessary churn. If we cannot find a clean way to reset unclaimed devices without duplicating driver specific code at the bus level, then let's just assume the drivers need to probe to reset them even if the device is not used on a particular board. One thing that's unclear to me for the DT case is how we'll bind the driver-specific parts of the reset code to the device, particularly in the case where there's no driver present. It should be possible to place an initcall in the driver-specific header, but that seems like a hack. Any thoughts on this? I think we can just initialize the driver-specific reset functions in hwmod code and those can just call the driver specific inline functions as needed in the driver specific header files. please don't. This will not work as you expect, we will just add more special cases to drivers which will be even more complex to maintain. What you want is a generic reset callback
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
On Tue, Feb 19, 2013 at 08:30:53AM -0800, Tony Lindgren wrote:
* Russell King - ARM Linux li...@arm.linux.org.uk [130219 07:49]:
If you want such things as pci_enable_device(), then what you're actually
asking for is omap_enable_device() for OMAP devices. OMAP devices are
already specific enough to OMAP SoCs (god knows, they have really complex
and obscure behaviours that no one else in their right mind would want
to copy) that calling out to omap specific functions would never really
be a problem.
I'd rather avoid adding omap_enable_device() calls to drivers as we really
want to keep the drivers generic. But maybe there could be some generic
bus_enable_device() function pointer that could be populated by the bus
code during init.
What you're not getting is that pci_enable_device() is a PCI thing which
is mostly a no-op - and where it isn't a no-op, it's something that must
be done _before_ PCI resources are used or even reserved (because
conceptually, this is to do with getting the resources correct.)
The PCI case is:
pci_device_probe(pci_dev)
{
pci_enable_device(pci_dev);
pci_request_regions(pci_dev);
regs = pci_iomap(pci_dev, BAR, size);
...
}
That's different from what you're wanting on OMAP - what you're wanting
there is some way to record the platform device has been ioremapped, so
that you can then fiddle with its idle/reset register from bus code.
If you think about it in light of the above sequence, the enable device
stage *doesn't* suit your needs because that happens before the driver
has done anything.
However... if you think you're going to get away with another total
rewrite of OMAP device support away from hwmod to a new scheme with a
new load of huge churn, think again. Remember, churn is evil. I've
complained to you about the amount of churn that OMAP manufactures
in the past. Linus has complained about it too. You can't continue
like this.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Tue, Feb 19, 2013 at 09:43:36AM -0800, Tony Lindgren wrote: On Tue, Feb 19, 2013 at 08:38:20AM -0800, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [130214 12:51]: On Thu, 14 Feb 2013, Tony Lindgren wrote: I don't think so as hwmod should only touch the sysconfig space when no driver has claimed it. hwmod does need to touch the SYSCONFIG register during pm_runtime suspend and resume operations, and during device enable and disable operations. Here's the relevant code: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2157 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/mach-omap2/omap_hwmod.c;h=4653efb87a2721ea20a9fe06a30a6c204d6d2282;hb=HEAD#l2195 But that's triggered by runtime PM from the device driver, right? Yes - for devices with drivers, it will hopefully be called by the driver. I think it's fine for hwmod to do that as requested by the device driver via runtime PM since hwmod is the bus glue making the drivers arch independent. I think the only piece we're missing for that is for driver to run something like pm_runtime_init() that initializes the address space to hwmod. In the current design, we might be able to do this during the driver's first call to pm_runtime_get*(). I think that's the first point that we can hook into the PM runtime code. Sounds doable and generic for now. this isn't really a nice way IMHO. You will end up with drivers assuming that certain details will be initialized whenever it calls pm_runtime_get() for the first time. Then, for devices such as UART which, when used as console, will not get pm_domain-runtime_resume() called because pm_runtime_set_active() has been called right before. IOW, this will not work for all cases and very soon we will have bug reports. Some generic way to init this from drivers is needed, I'm out of ideas here though. that's fair, but exposing driver-specific details via a static inline in a header which might or might not be called from hwmod isn't the best choice IMHO. the problems this could cause are numerous, specially when you consider drivers which are shared among multiple architectures (think musb, dwc3, ehci, ohci, xhci, ahci_platform, etc). Once the hwmod code is moved out to be a bus, I'm hoping we can do this through the driver making a dev-bus-enable_device() call - similar to the way PCI drivers call pci_enable_device(), but not bus-specific. That should remove our current dependency on CONFIG_PM_RUNTIME for OMAP devices to work correctly :-( Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. this wouldn't work. How would you write a reset for i2c which can be called for all instances even when there is no driver loaded ? What would be the arguments to such a call ? If you have something like: static inline void omap_i2c_reset(int bus_nr); you would need to keep track of the bus numbers even when there is no driver around and if there is a driver, you would have to ask i2c-omap.c to track all available instances and convert that number to the proper struct omap_i2c pointer. None of those sound like a good plan IMO. From a driver perspective, we want all our calls to have our private structure or a *dev as argument. Those are only avaiable when driver is around and anything which doesn't take those as argument will force driver to add extra unnecessary churn. If we cannot find a clean way to reset unclaimed devices without duplicating driver specific code at the bus level, then let's just assume the drivers need to probe to reset them even if the device is not used on a particular board. that wouldn't work either. Whenever you have a product which wants to optimize memory footprint, will start by disabling unused modules, then all of your problems are back and we will start to receive bug reports because of an artificial requirement
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
[...] Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. This thread is going in many directions and I've lost track. I think we need to separate out the specific reset cases and look at examples, since there are not very many (in theory). Specifically, IMO, we need to separate out the init-time reset needs from the runtime reset needs. It seems to me that init-time reset issues can (and should) be handled by omap_device/omap_hwmod. For devices with drivers, this can be done easily before driver probe (using bus notifiers). Any remaining devices that specifically need reset can be reset later (which devices are these?) The other problem is the where reset is need during runtime. Again, what are the specific examples here? The one I can think of off the top of my head is I2C, where it's needed in certain error recovery scenarios. Here, we need a way for the driver itself to initiate a full reset. Maybe a new runtime PM hook like -runtime_reset() as Felipe has suggested (though I'm not yet sure runtime PM is the right place for this, since it's not strictly related to runtime PM). Or, if these are mostly corner-case, error recovery scenarios, maybe a a way force the driver to remove itself and re-probe (which would trigger the bus-notifier based reset) as Tony has suggested is the better approach. Either way, it would be good to summarize the devices that need this to be sure what the actual needs are. The OMAP I2C driver is doing a reset and fully re-initializing itself, so it seems the forced remove/probe is the right approach there. Any one have the other use cases handy? Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
* Russell King - ARM Linux li...@arm.linux.org.uk [130219 10:26]:
On Tue, Feb 19, 2013 at 08:30:53AM -0800, Tony Lindgren wrote:
* Russell King - ARM Linux li...@arm.linux.org.uk [130219 07:49]:
If you want such things as pci_enable_device(), then what you're actually
asking for is omap_enable_device() for OMAP devices. OMAP devices are
already specific enough to OMAP SoCs (god knows, they have really complex
and obscure behaviours that no one else in their right mind would want
to copy) that calling out to omap specific functions would never really
be a problem.
I'd rather avoid adding omap_enable_device() calls to drivers as we really
want to keep the drivers generic. But maybe there could be some generic
bus_enable_device() function pointer that could be populated by the bus
code during init.
What you're not getting is that pci_enable_device() is a PCI thing which
is mostly a no-op - and where it isn't a no-op, it's something that must
be done _before_ PCI resources are used or even reserved (because
conceptually, this is to do with getting the resources correct.)
The PCI case is:
pci_device_probe(pci_dev)
{
pci_enable_device(pci_dev);
pci_request_regions(pci_dev);
regs = pci_iomap(pci_dev, BAR, size);
...
}
That's different from what you're wanting on OMAP - what you're wanting
there is some way to record the platform device has been ioremapped, so
that you can then fiddle with its idle/reset register from bus code.
If you think about it in light of the above sequence, the enable device
stage *doesn't* suit your needs because that happens before the driver
has done anything.
Right.. that won't help then. It sounds like the proper place would
be something like pm_runtime_init() as these register manage things
like autoidle etc.
However... if you think you're going to get away with another total
rewrite of OMAP device support away from hwmod to a new scheme with a
new load of huge churn, think again. Remember, churn is evil. I've
complained to you about the amount of churn that OMAP manufactures
in the past. Linus has complained about it too. You can't continue
like this.
I don't think there's any churn needed here if done properly. It's
mostly a question of dropping duplicate data from hwmod that we
already have available in device tree. That means we can shrink the
hwmod data needed.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Hi, On Tue, Feb 19, 2013 at 11:16:38AM -0800, Kevin Hilman wrote: [ ... ] The other problem is the where reset is need during runtime. Again, what are the specific examples here? The one I can think of off the top of my head is I2C, where it's needed in certain error recovery scenarios. right, I still have a theory that it's not needed in that case either, though I haven't had time to try that out. Here, we need a way for the driver itself to initiate a full reset. Maybe a new runtime PM hook like -runtime_reset() as Felipe has suggested (though I'm not yet sure runtime PM is the right place for this, since it's not strictly related to runtime PM). Or, if these are right, I agree with you but I couldn't think of a better place. Maybe we need a reset hook in struct device itself (as I suggested on another mail) but I'm not sure Greg would take it, unless we have a damn good reason. mostly corner-case, error recovery scenarios, maybe a a way force the driver to remove itself and re-probe (which would trigger the bus-notifier based reset) as Tony has suggested is the better approach. I don't think so. We certainly don't need to go through all memory allocations again. That will just cause unnecessary memory fragmentation. The OMAP I2C driver is doing a reset and fully re-initializing itself, so it seems the forced remove/probe is the right approach there. Any I must disagree here. Doing a reprobe of I2C every time there's an error condition is overkill. It means freeing struct omap_i2c_dev, freeing its IRQ, freeing the ioremapped area, changing mux configuration (due to pinctrl calls) just to do it all over again. Besides omap_i2c_probe() calls omap_i2c_init() which will do the full IP reset anyway. The difference is that calling omap_i2c_init() directly doesn't force us to free and reallocate all resources all over again. one have the other use cases handy? dwc3 gets reset during probe() too, but that has an IP specific reset which doesn't need SYSCONFIG register for that. -- balbi signature.asc Description: Digital signature
hwmod data duplication (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency)
Hi,
On Tue, Feb 19, 2013 at 11:31:22AM -0800, Tony Lindgren wrote:
However... if you think you're going to get away with another total
rewrite of OMAP device support away from hwmod to a new scheme with a
new load of huge churn, think again. Remember, churn is evil. I've
complained to you about the amount of churn that OMAP manufactures
in the past. Linus has complained about it too. You can't continue
like this.
I don't think there's any churn needed here if done properly. It's
mostly a question of dropping duplicate data from hwmod that we
already have available in device tree. That means we can shrink the
hwmod data needed.
how about starting by removing register addresses and interrupt numbers
which are passed by devicetree today ? If you want to move to DT-only
now even without all drivers DT-adapted, you can have something like
what Marvel folks did for kirkwood:
static void __init kirkwood_dt_init(void)
{
[ ... ]
if (of_machine_is_compatible(globalscale,dreamplug))
dreamplug_init();
if (of_machine_is_compatible(dlink,dns-kirkwood))
dnskw_init();
if (of_machine_is_compatible(iom,iconnect))
iconnect_init();
[ ... ]
of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
}
those machine-based inits are there just to initialize drivers which
aren't converted to DT.
this would let you remove all board-files except board-generic.c and
remove data which is already passed in via DT. It has the benefit of
showing Linus (or whoever cares) that we are working to remove the
churn while also removing some of the pressure of DT conversion, since
there are still details which need to be sorted out (UART function
pointers, clock nodes via DT - see Roger's discussion with Rajendra, DMA
nodes, etc etc).
Only on board-files we're talking about over 13K lines:
arch/arm/mach-omap2/board-2430sdp.c | 289 --
arch/arm/mach-omap2/board-3430sdp.c | 602
arch/arm/mach-omap2/board-3630sdp.c | 216 -
arch/arm/mach-omap2/board-4430sdp.c | 730 ---
arch/arm/mach-omap2/board-am3517crane.c | 97 --
arch/arm/mach-omap2/board-am3517evm.c| 398
arch/arm/mach-omap2/board-apollon.c | 342 ---
arch/arm/mach-omap2/board-cm-t35.c | 769
arch/arm/mach-omap2/board-cm-t3517.c | 302 --
arch/arm/mach-omap2/board-devkit8000.c | 648 -
arch/arm/mach-omap2/board-flash.c| 242 -
arch/arm/mach-omap2/board-flash.h| 62 --
arch/arm/mach-omap2/board-h4.c | 347 ---
arch/arm/mach-omap2/board-igep0020.c | 673 --
arch/arm/mach-omap2/board-ldp.c | 440 -
arch/arm/mach-omap2/board-n8x0.c | 762 ---
arch/arm/mach-omap2/board-omap3beagle.c | 549 ---
arch/arm/mach-omap2/board-omap3evm.c | 762 ---
arch/arm/mach-omap2/board-omap3logic.c | 249 -
arch/arm/mach-omap2/board-omap3pandora.c | 623 -
arch/arm/mach-omap2/board-omap3stalker.c | 432 -
arch/arm/mach-omap2/board-omap3touchbook.c | 391
arch/arm/mach-omap2/board-omap4panda.c | 467 --
arch/arm/mach-omap2/board-overo.c| 556 ---
arch/arm/mach-omap2/board-rm680.c| 165
arch/arm/mach-omap2/board-rx51-peripherals.c | 1276 --
arch/arm/mach-omap2/board-rx51-video.c | 89 --
arch/arm/mach-omap2/board-rx51.c | 128 ---
arch/arm/mach-omap2/board-rx51.h | 11 -
arch/arm/mach-omap2/board-ti8168evm.c| 62 --
arch/arm/mach-omap2/board-zoom-debugboard.c | 139 ---
arch/arm/mach-omap2/board-zoom-display.c | 147 ---
arch/arm/mach-omap2/board-zoom-peripherals.c | 304 --
arch/arm/mach-omap2/board-zoom.c | 155
arch/arm/mach-omap2/board-zoom.h | 10 -
35 files changed, 13434 deletions(-)
If we remove all addresses and interrupts, numbers look even better.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
Felipe Balbi ba...@ti.com writes: Hi, On Tue, Feb 19, 2013 at 11:16:38AM -0800, Kevin Hilman wrote: [ ... ] The other problem is the where reset is need during runtime. Again, what are the specific examples here? The one I can think of off the top of my head is I2C, where it's needed in certain error recovery scenarios. right, I still have a theory that it's not needed in that case either, though I haven't had time to try that out. Great, I hope it's not needed. I've asked several times in past as this driver was converted to runtime PM, but was told it was required for various reasons (which I can't remember anymore.) Here, we need a way for the driver itself to initiate a full reset. Maybe a new runtime PM hook like -runtime_reset() as Felipe has suggested (though I'm not yet sure runtime PM is the right place for this, since it's not strictly related to runtime PM). Or, if these are right, I agree with you but I couldn't think of a better place. Maybe we need a reset hook in struct device itself (as I suggested on another mail) but I'm not sure Greg would take it, unless we have a damn good reason. I'm starting to think a -hardreset() method in struct dev_pm_ops is the place to put this. IMO, it needs to be in dev_pm_ops, so it can be selectively overridden by PM domains. On OMAP, we'd just hook it up to omap_device_shutdown() for all omap_devices, and we'd be done. mostly corner-case, error recovery scenarios, maybe a a way force the driver to remove itself and re-probe (which would trigger the bus-notifier based reset) as Tony has suggested is the better approach. I don't think so. We certainly don't need to go through all memory allocations again. That will just cause unnecessary memory fragmentation. The OMAP I2C driver is doing a reset and fully re-initializing itself, so it seems the forced remove/probe is the right approach there. Any I must disagree here. Doing a reprobe of I2C every time there's an error condition is overkill. It means freeing struct omap_i2c_dev, freeing its IRQ, freeing the ioremapped area, changing mux configuration (due to pinctrl calls) just to do it all over again. Besides omap_i2c_probe() calls omap_i2c_init() which will do the full IP reset anyway. The difference is that calling omap_i2c_init() directly doesn't force us to free and reallocate all resources all over again. I agree, that would be a lot of churn. one have the other use cases handy? dwc3 gets reset during probe() too, but that has an IP specific reset which doesn't need SYSCONFIG register for that. OK, but that's still an init-time reset issue Do you know of any other runtime reset cases? If I2C is the only one, then maybe seeing if it can be removed is the right first step. If that works, we don't need any of this. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
OMAP reset requirements (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency) :x
Hi, On Tue, Feb 19, 2013 at 11:50:37AM -0800, Kevin Hilman wrote: The other problem is the where reset is need during runtime. Again, what are the specific examples here? The one I can think of off the top of my head is I2C, where it's needed in certain error recovery scenarios. right, I still have a theory that it's not needed in that case either, though I haven't had time to try that out. Great, I hope it's not needed. I've asked several times in past as this driver was converted to runtime PM, but was told it was required for various reasons (which I can't remember anymore.) :-) just like I2C's revision register was broken and had no useful data or MUSB mode1 couldn't be made to work... right. Looking at the functional spec, there's no mention that we need to driver reset signal anywhere, we _do_ need to reinitialize the transfer-related registers (I2C_CON, I2C_CNT, I2C_SA), other than that, I can't think of a need for driving the reset signal. The bus specification isn't that stupid right ? Arbitration Lost and NACK are both expected bus conditions. My only concern is write underflow and read overflow, but I haven't been able to force that to happen yet. Even in those cases, I'd be very surprised if we really needed to driver the reset signal. Here, we need a way for the driver itself to initiate a full reset. Maybe a new runtime PM hook like -runtime_reset() as Felipe has suggested (though I'm not yet sure runtime PM is the right place for this, since it's not strictly related to runtime PM). Or, if these are right, I agree with you but I couldn't think of a better place. Maybe we need a reset hook in struct device itself (as I suggested on another mail) but I'm not sure Greg would take it, unless we have a damn good reason. I'm starting to think a -hardreset() method in struct dev_pm_ops is the place to put this. IMO, it needs to be in dev_pm_ops, so it can be selectively overridden by PM domains. On OMAP, we'd just hook it up to omap_device_shutdown() for all omap_devices, and we'd be done. do you mean omap_device_shutdown(); omap_device_enable(); ? one have the other use cases handy? dwc3 gets reset during probe() too, but that has an IP specific reset which doesn't need SYSCONFIG register for that. OK, but that's still an init-time reset issue yeah, and this is pretty mandatory otherwise dwc3 and its PHYs (USB3 pair plus USB2) won't synchronize and we'll have all sorts of issues :-) Do you know of any other runtime reset cases? If I2C is the only one, then maybe seeing if it can be removed is the right first step. If that works, we don't need any of this. I don't know of any other, but there might be a few... On the other hand, I doubt there's any of them which is truly necessary, unless there's a bug in the IP which we can solve by issuing a reset, other than that, driving the reset signal shouldn't be necessary outside of probe() (or something before probe()). -- balbi signature.asc Description: Digital signature
Re: OMAP reset requirements
Felipe Balbi ba...@ti.com writes: Hi, On Tue, Feb 19, 2013 at 11:50:37AM -0800, Kevin Hilman wrote: The other problem is the where reset is need during runtime. Again, what are the specific examples here? The one I can think of off the top of my head is I2C, where it's needed in certain error recovery scenarios. right, I still have a theory that it's not needed in that case either, though I haven't had time to try that out. Great, I hope it's not needed. I've asked several times in past as this driver was converted to runtime PM, but was told it was required for various reasons (which I can't remember anymore.) :-) just like I2C's revision register was broken and had no useful data or MUSB mode1 couldn't be made to work... right. Looking at the functional spec, there's no mention that we need to driver reset signal anywhere, we _do_ need to reinitialize the transfer-related registers (I2C_CON, I2C_CNT, I2C_SA), other than that, I can't think of a need for driving the reset signal. The bus specification isn't that stupid right ? Arbitration Lost and NACK are both expected bus conditions. My only concern is write underflow and read overflow, but I haven't been able to force that to happen yet. Even in those cases, I'd be very surprised if we really needed to driver the reset signal. IIRC, it's definitely related to recovering from underflow/overflow. Here, we need a way for the driver itself to initiate a full reset. Maybe a new runtime PM hook like -runtime_reset() as Felipe has suggested (though I'm not yet sure runtime PM is the right place for this, since it's not strictly related to runtime PM). Or, if these are right, I agree with you but I couldn't think of a better place. Maybe we need a reset hook in struct device itself (as I suggested on another mail) but I'm not sure Greg would take it, unless we have a damn good reason. I'm starting to think a -hardreset() method in struct dev_pm_ops is the place to put this. IMO, it needs to be in dev_pm_ops, so it can be selectively overridden by PM domains. On OMAP, we'd just hook it up to omap_device_shutdown() for all omap_devices, and we'd be done. do you mean omap_device_shutdown(); omap_device_enable(); ? Possibly, depending on the current state of the device. That method would have to be smart enough to know the current state of the device, and return with it in the same state so as not to confuse runtime PM usecounting. one have the other use cases handy? dwc3 gets reset during probe() too, but that has an IP specific reset which doesn't need SYSCONFIG register for that. OK, but that's still an init-time reset issue yeah, and this is pretty mandatory otherwise dwc3 and its PHYs (USB3 pair plus USB2) won't synchronize and we'll have all sorts of issues :-) Do you know of any other runtime reset cases? If I2C is the only one, then maybe seeing if it can be removed is the right first step. If that works, we don't need any of this. I don't know of any other, but there might be a few... On the other hand, I doubt there's any of them which is truly necessary, unless there's a bug in the IP which we can solve by issuing a reset, other than that, driving the reset signal shouldn't be necessary outside of probe() (or something before probe()). If that's the case, then the approach should be to focus on sorting out the drivers that actually need to reset outside of init/probe before going and trying to extend the driver model. Combined with solving the init/probe time reset using bus notifiers (for devices with drivers) and late initcall for the rest should solve all the problems, I think. Am I missing anything? Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] reworked omap usb pdata changes for v3.9 merge window
On Friday 15 February 2013, Tony Lindgren wrote: Hi Arnd Olof, Roger Quadros reworked the OMAP USB patches that were causing a conflict in Linux next and we requested to be dropped from Linux next and reworked to remove the dependencies between core SoC code and the driver code. Below is a pull request for the mach-omap2 specific parts, the rest is driver specific and queued by Samuel in the MFD tree and Greg for the USB changes. I've left it unresolved so you can merge it into one of your branches as a dependency so we can avoid the merge conflict in Linux next. A link to the merge resolution done by Stephen is in the pull request. Let me know if you want me to resolve it instead. I've pulled them into next/soc now. I was wondering whether to put them into the late/omap branch instead, but this seemed like the easiest way to resolve the conflict, and the patches are simple enough. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] reworked omap usb pdata changes for v3.9 merge window
* Arnd Bergmann a...@arndb.de [130219 13:34]: On Friday 15 February 2013, Tony Lindgren wrote: Hi Arnd Olof, Roger Quadros reworked the OMAP USB patches that were causing a conflict in Linux next and we requested to be dropped from Linux next and reworked to remove the dependencies between core SoC code and the driver code. Below is a pull request for the mach-omap2 specific parts, the rest is driver specific and queued by Samuel in the MFD tree and Greg for the USB changes. I've left it unresolved so you can merge it into one of your branches as a dependency so we can avoid the merge conflict in Linux next. A link to the merge resolution done by Stephen is in the pull request. Let me know if you want me to resolve it instead. I've pulled them into next/soc now. I was wondering whether to put them into the late/omap branch instead, but this seemed like the easiest way to resolve the conflict, and the patches are simple enough. OK thanks! Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hwmod data duplication (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency)
* Felipe Balbi ba...@ti.com [130219 11:47]:
Hi,
On Tue, Feb 19, 2013 at 11:31:22AM -0800, Tony Lindgren wrote:
However... if you think you're going to get away with another total
rewrite of OMAP device support away from hwmod to a new scheme with a
new load of huge churn, think again. Remember, churn is evil. I've
complained to you about the amount of churn that OMAP manufactures
in the past. Linus has complained about it too. You can't continue
like this.
I don't think there's any churn needed here if done properly. It's
mostly a question of dropping duplicate data from hwmod that we
already have available in device tree. That means we can shrink the
hwmod data needed.
how about starting by removing register addresses and interrupt numbers
which are passed by devicetree today ? If you want to move to DT-only
now even without all drivers DT-adapted, you can have something like
what Marvel folks did for kirkwood:
static void __init kirkwood_dt_init(void)
{
[ ... ]
if (of_machine_is_compatible(globalscale,dreamplug))
dreamplug_init();
if (of_machine_is_compatible(dlink,dns-kirkwood))
dnskw_init();
if (of_machine_is_compatible(iom,iconnect))
iconnect_init();
[ ... ]
of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
}
those machine-based inits are there just to initialize drivers which
aren't converted to DT.
Yes we could do that at least partially, but..
this would let you remove all board-files except board-generic.c and
remove data which is already passed in via DT. It has the benefit of
showing Linus (or whoever cares) that we are working to remove the
churn while also removing some of the pressure of DT conversion, since
there are still details which need to be sorted out (UART function
pointers, clock nodes via DT - see Roger's discussion with Rajendra, DMA
nodes, etc etc).
..that means massive amount of churn in the board-*.c files to convert
them to various init functions to be called from board-generic.c and
removing the ones that are working with DT.
I think we're better off making first sure things are usable with
DT, then just dropping the board-*.c files as we go.
And omap4 is the place to start as we only have blaze and panda
board files. Once DSS, USB and WLAN work with the .dts files, we
can just drop those board files and make omap4 DT only.
We may be able to drop omap4 board-*.c files faster than going full
DT with few selected legacy init functions in board-generic.c for
things like LCD panel configuration etc.
Only on board-files we're talking about over 13K lines:
arch/arm/mach-omap2/board-2430sdp.c | 289 --
arch/arm/mach-omap2/board-3430sdp.c | 602
arch/arm/mach-omap2/board-3630sdp.c | 216 -
arch/arm/mach-omap2/board-4430sdp.c | 730 ---
arch/arm/mach-omap2/board-am3517crane.c | 97 --
arch/arm/mach-omap2/board-am3517evm.c| 398
arch/arm/mach-omap2/board-apollon.c | 342 ---
arch/arm/mach-omap2/board-cm-t35.c | 769
arch/arm/mach-omap2/board-cm-t3517.c | 302 --
arch/arm/mach-omap2/board-devkit8000.c | 648 -
arch/arm/mach-omap2/board-flash.c| 242 -
arch/arm/mach-omap2/board-flash.h| 62 --
arch/arm/mach-omap2/board-h4.c | 347 ---
arch/arm/mach-omap2/board-igep0020.c | 673 --
arch/arm/mach-omap2/board-ldp.c | 440 -
arch/arm/mach-omap2/board-n8x0.c | 762 ---
arch/arm/mach-omap2/board-omap3beagle.c | 549 ---
arch/arm/mach-omap2/board-omap3evm.c | 762 ---
arch/arm/mach-omap2/board-omap3logic.c | 249 -
arch/arm/mach-omap2/board-omap3pandora.c | 623 -
arch/arm/mach-omap2/board-omap3stalker.c | 432 -
arch/arm/mach-omap2/board-omap3touchbook.c | 391
arch/arm/mach-omap2/board-omap4panda.c | 467 --
arch/arm/mach-omap2/board-overo.c| 556 ---
arch/arm/mach-omap2/board-rm680.c| 165
arch/arm/mach-omap2/board-rx51-peripherals.c | 1276
--
arch/arm/mach-omap2/board-rx51-video.c | 89 --
arch/arm/mach-omap2/board-rx51.c | 128 ---
arch/arm/mach-omap2/board-rx51.h | 11 -
arch/arm/mach-omap2/board-ti8168evm.c| 62 --
arch/arm/mach-omap2/board-zoom-debugboard.c | 139 ---
arch/arm/mach-omap2/board-zoom-display.c | 147 ---
arch/arm/mach-omap2/board-zoom-peripherals.c | 304 --
arch/arm/mach-omap2/board-zoom.c | 155
arch/arm/mach-omap2/board-zoom.h | 10 -
35 files changed, 13434 deletions(-)
If we remove all addresses and interrupts, numbers
Re: hwmod data duplication (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency)
Hi,
On Tue, Feb 19, 2013 at 02:09:33PM -0800, Tony Lindgren wrote:
On Tue, Feb 19, 2013 at 11:31:22AM -0800, Tony Lindgren wrote:
However... if you think you're going to get away with another total
rewrite of OMAP device support away from hwmod to a new scheme with a
new load of huge churn, think again. Remember, churn is evil. I've
complained to you about the amount of churn that OMAP manufactures
in the past. Linus has complained about it too. You can't continue
like this.
I don't think there's any churn needed here if done properly. It's
mostly a question of dropping duplicate data from hwmod that we
already have available in device tree. That means we can shrink the
hwmod data needed.
how about starting by removing register addresses and interrupt numbers
which are passed by devicetree today ? If you want to move to DT-only
now even without all drivers DT-adapted, you can have something like
what Marvel folks did for kirkwood:
static void __init kirkwood_dt_init(void)
{
[ ... ]
if (of_machine_is_compatible(globalscale,dreamplug))
dreamplug_init();
if (of_machine_is_compatible(dlink,dns-kirkwood))
dnskw_init();
if (of_machine_is_compatible(iom,iconnect))
iconnect_init();
[ ... ]
of_platform_populate(NULL, kirkwood_dt_match_table, NULL, NULL);
}
those machine-based inits are there just to initialize drivers which
aren't converted to DT.
Yes we could do that at least partially, but..
this would let you remove all board-files except board-generic.c and
remove data which is already passed in via DT. It has the benefit of
showing Linus (or whoever cares) that we are working to remove the
churn while also removing some of the pressure of DT conversion, since
there are still details which need to be sorted out (UART function
pointers, clock nodes via DT - see Roger's discussion with Rajendra, DMA
nodes, etc etc).
..that means massive amount of churn in the board-*.c files to convert
them to various init functions to be called from board-generic.c and
removing the ones that are working with DT.
why ? I meant that only what's not converted to DT today should be
handled this way. Also, most of the churn is already there
(usb_musb_init(), usb_ehci_init(), etc etc), it just needs to be called
from a different place. We don't need to have one function for each
board, however, maybe we could target by-soc:
if (of_is_compatible(omap3))
omap3_init_devices(); /* or whatever you wanna call it */
omap_init_devices() has initialization for everything which isn't DT
adapted today and as we move things to DT, there's a single place to
remove code from.
I think we're better off making first sure things are usable with
DT, then just dropping the board-*.c files as we go.
And omap4 is the place to start as we only have blaze and panda
board files. Once DSS, USB and WLAN work with the .dts files, we
can just drop those board files and make omap4 DT only.
fair enough.
We may be able to drop omap4 board-*.c files faster than going full
DT with few selected legacy init functions in board-generic.c for
things like LCD panel configuration etc.
Only on board-files we're talking about over 13K lines:
arch/arm/mach-omap2/board-2430sdp.c | 289 --
arch/arm/mach-omap2/board-3430sdp.c | 602
arch/arm/mach-omap2/board-3630sdp.c | 216 -
arch/arm/mach-omap2/board-4430sdp.c | 730 ---
arch/arm/mach-omap2/board-am3517crane.c | 97 --
arch/arm/mach-omap2/board-am3517evm.c| 398
arch/arm/mach-omap2/board-apollon.c | 342 ---
arch/arm/mach-omap2/board-cm-t35.c | 769
arch/arm/mach-omap2/board-cm-t3517.c | 302 --
arch/arm/mach-omap2/board-devkit8000.c | 648 -
arch/arm/mach-omap2/board-flash.c| 242 -
arch/arm/mach-omap2/board-flash.h| 62 --
arch/arm/mach-omap2/board-h4.c | 347 ---
arch/arm/mach-omap2/board-igep0020.c | 673 --
arch/arm/mach-omap2/board-ldp.c | 440 -
arch/arm/mach-omap2/board-n8x0.c | 762 ---
arch/arm/mach-omap2/board-omap3beagle.c | 549 ---
arch/arm/mach-omap2/board-omap3evm.c | 762 ---
arch/arm/mach-omap2/board-omap3logic.c | 249 -
arch/arm/mach-omap2/board-omap3pandora.c | 623 -
arch/arm/mach-omap2/board-omap3stalker.c | 432 -
arch/arm/mach-omap2/board-omap3touchbook.c | 391
arch/arm/mach-omap2/board-omap4panda.c | 467 --
arch/arm/mach-omap2/board-overo.c| 556 ---
arch/arm/mach-omap2/board-rm680.c| 165
Re: hwmod data duplication (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency)
* Felipe Balbi ba...@ti.com [130219 14:26]: On Tue, Feb 19, 2013 at 02:09:33PM -0800, Tony Lindgren wrote: ..that means massive amount of churn in the board-*.c files to convert them to various init functions to be called from board-generic.c and removing the ones that are working with DT. why ? I meant that only what's not converted to DT today should be handled this way. Also, most of the churn is already there (usb_musb_init(), usb_ehci_init(), etc etc), it just needs to be called from a different place. We don't need to have one function for each board, however, maybe we could target by-soc: if (of_is_compatible(omap3)) omap3_init_devices(); /* or whatever you wanna call it */ omap_init_devices() has initialization for everything which isn't DT adapted today and as we move things to DT, there's a single place to remove code from. And the pdata for that comes from where? :) I think that means converting each board-*.c to device init functions, which leads to the churn I was mentioning.. I think we're better off making first sure things are usable with DT, then just dropping the board-*.c files as we go. And omap4 is the place to start as we only have blaze and panda board files. Once DSS, USB and WLAN work with the .dts files, we can just drop those board files and make omap4 DT only. fair enough. We may be able to drop omap4 board-*.c files faster than going full DT with few selected legacy init functions in board-generic.c for things like LCD panel configuration etc. Only on board-files we're talking about over 13K lines: 35 files changed, 13434 deletions(-) ... If we remove all addresses and interrupts, numbers look even better. Yeah. Let's start with omap4 first when DSS + USB + WLAN work. USB is going to be ready for v3.10, likely Wlan too. That's good news. Maybe we can then have a legacy device pdata init for DSS, and make omap4 DT only for v3.10. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hwmod data duplication (was: Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency)
Hi, On Tue, Feb 19, 2013 at 02:31:28PM -0800, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [130219 14:26]: On Tue, Feb 19, 2013 at 02:09:33PM -0800, Tony Lindgren wrote: ..that means massive amount of churn in the board-*.c files to convert them to various init functions to be called from board-generic.c and removing the ones that are working with DT. why ? I meant that only what's not converted to DT today should be handled this way. Also, most of the churn is already there (usb_musb_init(), usb_ehci_init(), etc etc), it just needs to be called from a different place. We don't need to have one function for each board, however, maybe we could target by-soc: if (of_is_compatible(omap3)) omap3_init_devices(); /* or whatever you wanna call it */ omap_init_devices() has initialization for everything which isn't DT adapted today and as we move things to DT, there's a single place to remove code from. And the pdata for that comes from where? :) I think that means converting each board-*.c to device init functions, which leads to the churn I was mentioning.. fair enough, still a lot less churn ;-) -- balbi signature.asc Description: Digital signature
Re: Guidelines to move mach-omap2/gpmc to drivers/memory-controller
Hi Jon, On Fri, Feb 15, 2013 at 02:08:08PM -0600, Jon Hunter wrote: On 02/15/2013 01:53 PM, Paul Walmsley wrote: Hi, On Fri, 15 Feb 2013, Ezequiel Garcia wrote: I imagine one of the biggest issues is GPMC's dependency on hwmod code. Can anyone shed some light on how to handle this? What dependency is this? I'm not aware of any GPMC dependency to hwmod; there shouldn't be any. If Ezequiel is referring to the code in omap_gpmc_init() that is looking up the hwmod struct for gpmc, then this not a dependency. This function should remain in mach-omap2/gpmc.c and not be moved into drivers/. Yes, I was referring to that code. Eventually once we have migrated to DT, this function will be removed altogether. Mmm, I see. So I might try to move GPMC and friends to drivers/memory keeping the hwmod code inside mach-omap2. I already did a few tests and I **think** that with a little luck we can have a not-so-intrusive patch that moves GPMC. @Jon: Do you think it's worth the effort? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/NOT FOR MERGING 2/3] serial: omap: remove hwmod dependency
On Wednesday 20 February 2013 12:46 AM, Kevin Hilman wrote: [...] Just to recap what we've discussed earlier, the reasons why we want reset and idle functions should be in the driver specific header are: 1. There's often driver specific logic needed in addition to the syconfig tinkering in the reset/idle functions. It's been a while since I last tabulated this. But my recollection was that some kind of IP block-specific reset code is needed for about 7% to 10% of the OMAP IP blocks. Yes it's not too many, but the issue there is the driver specific code that's duplicated in both places. And sounds like the solution to that is to make driver specific reset code a static inline function in the driver header as discussed earlier so bus code can call it when there's no driver loaded. This thread is going in many directions and I've lost track. Indeed. I lost track too and almost gave up further reading. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
