Re: [PATCH v2 1/8] ARM: DRA7: id: Add cpu detection support for DRA7xx based SoCs'
On Wednesday 31 July 2013 12:13 AM, Nishanth Menon wrote:
On 07/30/2013 01:37 PM, Sricharan R wrote:
Hi,
On Tuesday 30 July 2013 09:02 PM, Felipe Balbi wrote:
Hi,
On Tue, Jul 30, 2013 at 08:06:31PM +0530, Sricharan R wrote:
On Tuesday 30 July 2013 07:53 PM, Felipe Balbi wrote:
Hi,
On Tue, Jul 30, 2013 at 07:48:23PM +0530, Sricharan R wrote:
On Tuesday 30 July 2013 06:40 PM, Felipe Balbi wrote:
Hi,
On Tue, Jul 30, 2013 at 04:55:39PM +0530, Rajendra Nayak wrote:
@@ -379,6 +407,13 @@ IS_OMAP_TYPE(3430, 0x3430)
# define soc_is_omap543x()is_omap543x()
#endif
+# if defined(CONFIG_SOC_DRA7XX)
+# undef soc_is_dra7xx
+# undef soc_is_dra75x
+# define soc_is_dra7xx()is_dra7xx()
+# define soc_is_dra75x()is_dra75x()
since this platform is DT-only, couldn't we just believe DT-data to be
correct of_machine_is_compatible() ? 2/3 of this patch would be removed.
I patched this for OMAP5 (compile-tested only, no boards available) and
came out with the patch below (still needs to be split):
diff --git a/arch/arm/boot/dts/omap5-uevm.dts
b/arch/arm/boot/dts/omap5-uevm.dts
index 08b7267..b3136e5 100644
--- a/arch/arm/boot/dts/omap5-uevm.dts
+++ b/arch/arm/boot/dts/omap5-uevm.dts
@@ -13,7 +13,7 @@
/ {
model = TI OMAP5 uEVM board;
-compatible = ti,omap5-uevm, ti,omap5;
+compatible = ti,omap5-uevm, ti,omap5432-es2.0, ti,omap5;
ok, nice and simpler way.
But would this make different revisions, to appear the same ?
well omap5-uevm is omap5432 es2.0 only, right ? If a new board comes up,
it should be treated as such, then you can pass a different string to
that new board's compatible attribute.
s
Yes for OMAP5. I was thinking in general about this approach.
For example, for OMAP4 we have same board and
different revisions can be socketed there.
For OMAP5, this is good.
do we really production socketed boards? Well, at least Blaze has such
thing. But do we have too many differences that need to be trated at
arch/arm or should/could those be handled by reading IP's revision
register (e.g. usb host erratas)
OMAP4 SDP is socketed as well.
a) OMAP4SDP is not production device
b) OMAP4SDP uses SOM (System On Module)
c) Socketted SOMs were used only during initial days of SoC
d) almost all latest OMAP4 SDP switched to using soldered in SOM
e) we claim compatibility of OMAP4 SDP with Blaze.
So, I dont think this is a rational argument for keeping soc checks with dts.
What about OMAP4 pandas? I for instance, have an old 4430 panda and I have no
idea
if its a es2.1 or a es2.3 or something else. If we start relying on dt to pass
the
right revision check then (we need to create different dts files for these to
begin with) I
need to know exactly what silicon rev I am running on.
I know its good to completely get rid of all silicon rev checks and depend on
IP revisions
but we have had various IPs which do not have proper rev checks. We have I
guess most often
used these to identify PRCM differences.
Tony, what do you suggest we do for this series? Since we have just an es1.0
and one board
at this point for dra7xx, things would be fine even if we do a dt based parsing
to identify
the device, and I am fine with it if thats what we feel is the right way
forward.
For the rest of the DT only platforms (omap4/5/am335x) anyway getting rid of
these rev checks
from the kernel and depending on DT parsing needs to be a separate series
anyway and I dont
plan to address those as part of this series.
Ya, revision checks used only in few places and as you said
we handle them using IP revisions, but that we have to look and clean
up those places, if we really intend to do this for other socs.
I agree this is the right approach :).
--
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 01/15] drivers: phy: add generic PHY framework
Hi,
On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:
IMHO we need a lookup method for PHYs, just like for clocks,
regulators, PWMs or even i2c busses because there are complex cases
when passing just a name using platform data will not work. I would
second what Stephen said [1] and define a structure doing things in a
DT-like way.
Example;
[platform code]
static const struct phy_lookup my_phy_lookup[] = {
PHY_LOOKUP(s3c-hsotg.0, otg, samsung-usbphy.1, phy.2),
The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
creating the device, the ids in the device name would change and
PHY_LOOKUP wont be useful.
I don't think this is a problem. All the existing lookup methods already
use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You
can simply add a requirement that the ID must be assigned manually,
without using PLATFORM_DEVID_AUTO to use PHY lookup.
And I'm saying that this idea, of using a specific name and id, is
frought with fragility and will break in the future in various ways when
devices get added to systems, making these strings constantly have to be
kept up to date with different board configurations.
People, NEVER, hardcode something like an id. The fact that this
happens today with the clock code, doesn't make it right, it makes the
clock code wrong. Others have already said that this is wrong there as
well, as systems change and dynamic ids get used more and more.
Let's not repeat the same mistakes of the past just because we refuse to
learn from them...
So again, the find a phy by a string functions should be removed, the
device id should be automatically created by the phy core just to make
things unique in sysfs, and no driver code should _ever_ be reliant on
the number that is being created, and the pointer to the phy structure
should be used everywhere instead.
With those types of changes, I will consider merging this subsystem, but
without them, sorry, I will not.
I'll agree with Greg here, the very fact that we see people trying to
add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points to a
big problem in the framework.
The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
adding similar infrastructure to the driver themselves to make sure we
don't end up with duplicate names in sysfs in case we have multiple
instances of the same IP in the SoC (or several of the same PCIe card).
I really don't want to go back to that.
If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can give the
correct binding information to the PHY framework. I think we can drop having
this non-dt support in PHY framework? I see only one platform (OMAP3) going to
be needing this non-dt support and we can use the USB PHY library for it.
you shouldn't drop support for non-DT platform, in any case we lived
without DT (and still do) for years. Gotta find a better way ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv4 29/33] CLK: omap: add omap3 clock init file
* Nishanth Menon n...@ti.com [130730 13:26]: On 07/23/2013 02:20 AM, Tero Kristo wrote: clk-3xxx.c now contains the clock init functionality for omap3, including DT clock registration and adding of static clkdev entries. This patch also splits the OMAP3 clock registration code under mach-omap2 to use OMAP3 subtype specific clk init functions. Signed-off-by: Tero Kristo t-kri...@ti.com --- arch/arm/mach-omap2/Makefile |2 +- arch/arm/mach-omap2/cclock3xxx_data.c | 3641 - Tony and a lot of people is not going to like removing support for non-dt boot for OMAP3 before it's time is due ;) I think the only showstopper for that is that we need the pending pinctrl changes merged first to keep off-idle working. So the omap3 legacy code removal probably needs to wait until v3.13 merge window. For omap4 and am33xx we can do it now. 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: [PATCH v2 1/8] ARM: DRA7: id: Add cpu detection support for DRA7xx based SoCs'
* Rajendra Nayak rna...@ti.com [130730 23:09]: Tony, what do you suggest we do for this series? Since we have just an es1.0 and one board at this point for dra7xx, things would be fine even if we do a dt based parsing to identify the device, and I am fine with it if thats what we feel is the right way forward. For the rest of the DT only platforms (omap4/5/am335x) anyway getting rid of these rev checks from the kernel and depending on DT parsing needs to be a separate series anyway and I dont plan to address those as part of this series. Well I'd say there's no need to drop the hardware revision checks at this point at least for existing hardware. That's a very minimal piece of code and there are way bigger issues to tackle. For new SoCs, we could do it based on the compatible flag. If it helps booting newer hardware with older kernels, then that's a good reason to do it. 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: [PATCH v2 1/8] ARM: DRA7: id: Add cpu detection support for DRA7xx based SoCs'
On Wednesday 31 July 2013 12:12 PM, Tony Lindgren wrote: * Rajendra Nayak rna...@ti.com [130730 23:09]: Tony, what do you suggest we do for this series? Since we have just an es1.0 and one board at this point for dra7xx, things would be fine even if we do a dt based parsing to identify the device, and I am fine with it if thats what we feel is the right way forward. For the rest of the DT only platforms (omap4/5/am335x) anyway getting rid of these rev checks from the kernel and depending on DT parsing needs to be a separate series anyway and I dont plan to address those as part of this series. Well I'd say there's no need to drop the hardware revision checks at this point at least for existing hardware. That's a very minimal piece of code and there are way bigger issues to tackle. right, makes sense. For new SoCs, we could do it based on the compatible flag. If it helps booting newer hardware with older kernels, then that's a good reason to do it. Sure, we can have dra7xx use the compatible flag and not add all the rev checks. That said, I would be glad if the latest kernels at least boot on newer hardware let alone older kernels :) But I guess we have bigger issues to tackle before even that happens. Thanks for the quick response. regards, Rajendra 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: Linux kernel for OMAP5432 uEVM
On Jul 30, 2013, at 11:52 AM, Lokesh Vutla lokeshvu...@ti.com wrote: You can also use Linus's kernel with the above clk data branch.( OMAP5 uEVM boots) Please let me know if you need more info. And the ethernet driver is not available by default? Cheers, Baozi-- 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: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
Hi,
On Wed, Jul 31, 2013 at 11:17:52AM +0530, Sourav Poddar wrote:
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
new file mode 100644
index 000..3d10b69
--- /dev/null
+++ b/drivers/spi/spi-ti-qspi.c
@@ -0,0 +1,545 @@
snip
+/* Device Control */
+#define QSPI_DD(m, n)(m (3 + n*8))
+#define QSPI_CKPHA(n)(1 (2 + n*8))
+#define QSPI_CSPOL(n)(1 (1 + n*8))
+#define QSPI_CKPOL(n)(1 (n*8))
add spaces around the * operator
+#define QSPI_FRAME_MAX 0xfff
Frame max is 4096, 0x1000, right ?
+static inline void ti_qspi_read_data(struct ti_qspi *qspi,
+ unsigned long reg, int wlen, u8 **rxbuf)
+{
+ switch (wlen) {
+ case 8:
+ **rxbuf = readb(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %02x\n, *(*rxbuf));
+ *rxbuf += 1;
+ break;
+ case 16:
+ **rxbuf = readw(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %04x\n, *(*rxbuf));
+ *rxbuf += 2;
+ break;
+ case 32:
+ **rxbuf = readl(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %04x\n, *(*rxbuf));
%08x, this was commented before.
+static int ti_qspi_setup(struct spi_device *spi)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi-master);
+ struct ti_qspi_regs *ctx_reg = qspi-ctx_reg;
+ int clk_div = 0, ret;
+ u32 clk_ctrl_reg, clk_rate, clk_mask;
+
+ clk_rate = clk_get_rate(qspi-fclk);
+
+ if (!qspi-spi_max_frequency) {
+ dev_err(qspi-dev, spi max frequency not defined\n);
+ return -EINVAL;
+ }
+
+ clk_div = DIV_ROUND_UP(clk_rate, qspi-spi_max_frequency) - 1;
+
+ dev_dbg(qspi-dev, %s: hz: %d, clock divider %d\n, __func__,
+ qspi-spi_max_frequency, clk_div);
+
+ ret = pm_runtime_get_sync(qspi-dev);
+ if (ret) {
+ dev_err(qspi-dev, pm_runtime_get_sync() failed\n);
+ return ret;
+ }
+
+ clk_ctrl_reg = ti_qspi_read(qspi, QSPI_SPI_CLOCK_CNTRL_REG);
+
+ clk_ctrl_reg = ~QSPI_CLK_EN;
+
+ if (spi-master-busy) {
+ dev_dbg(qspi-dev, master busy doing other trasnfers\n);
+ return -EBUSY;
+ }
this check can be done before pm_runtime_get_sync(), you're also leaking
pm_runtime reference here.
+ /* disable SCLK */
+ ti_qspi_write(qspi, clk_ctrl_reg, QSPI_SPI_CLOCK_CNTRL_REG);
+
+ if (clk_div 0) {
+ dev_dbg(qspi-dev, %s: clock divider 0, using /1 divider\n,
+ __func__);
+ pm_runtime_put_sync(qspi-dev);
+ return -EINVAL;
+ }
+
+ if (clk_div QSPI_CLK_DIV_MAX) {
+ dev_dbg(qspi-dev, %s: clock divider %d , using /%d
divider\n,
+ __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1);
+ pm_runtime_put_sync(qspi-dev);
+ return -EINVAL;
+ }
why don't you move all checks to clk_div before pm_runtime_get_sync()
call ?
+static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ const u8 *txbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ txbuf = t-tx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
you're decrementing count by one, but in some cases you write 4 bytes or
2 bytes... This will blow up very soon. I can already see overflows
happening...
+static int qspi_read_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ u8 *rxbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ rxbuf = t-rx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
ditto
+static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ int ret;
+
+ if (t-tx_buf) {
+ ret = qspi_write_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi-dev, Error while writing\n);
+ return -EINVAL;
why do you change the return value from qspi_write_msg() ?
+ }
+ }
+
+ if (t-rx_buf) {
+ ret = qspi_read_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi-dev, Error while writing\n);
+ return -EINVAL;
why do you change the return value from qspi_read_msg() ?
+static int ti_qspi_start_transfer_one(struct spi_master *master,
+ struct spi_message *m)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(master);
+ struct spi_device *spi = m-spi;
+ struct spi_transfer *t;
+ int status = 0, ret;
+ int frame_length;
+
+ /* setup device control reg */
+ qspi-dc = 0;
+
+ if (spi-mode SPI_CPHA)
+ qspi-dc |= QSPI_CKPHA(spi-chip_select);
+ if (spi-mode SPI_CPOL)
+ qspi-dc |= QSPI_CKPOL(spi-chip_select);
+ if
Re: [GIT PULL] ARM: OMAP2+: PRCM: drop macros not currently in use
* Paul Walmsley p...@pwsan.com [130721 22:02]: Hi Tony, Sorry for the delay on this - wanted to make sure it passed the local testbed before sending it upstream. The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b: Linux 3.11-rc2 (2013-07-21 12:05:29 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/pjw/omap-pending.git tags/for-v3.11-rc/omap-fixes-a for you to fetch changes up to 11dab344053f726fe17ede95aa52c1eea1258a66: ARM: OMAP5: PRM/CM: Cleanup unused header (2013-07-21 22:10:25 -0600) This series removes the currently-unused PRCM macros from arch/arm/mach-omap2. Thanks sorry for the delay on my part too, I was offline last week. I'll pull these into omap-for-v3.12/cleanup. 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: Linux kernel for OMAP5432 uEVM
Hi Chen, On Wednesday 31 July 2013 01:19 PM, Chen Baozi wrote: On Jul 30, 2013, at 11:52 AM, Lokesh Vutla lokeshvu...@ti.com wrote: You can also use Linus's kernel with the above clk data branch.( OMAP5 uEVM boots) Please let me know if you need more info. And the ethernet driver is not available by default? You need the following patch series recently posted by Roger: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92694.html Thanks and regards, Lokesh Cheers, Baozi -- 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: interrupt consistency check for OF GPIO IRQs
On 07/31/2013 01:44 AM, Linus Walleij wrote: On Tue, Jul 30, 2013 at 6:30 AM, Grant Likely grant.lik...@linaro.org wrote: On Mon, Jul 29, 2013 at 6:36 AM, Linus Walleij linus.wall...@linaro.org wrote: To solve this dilemma, perform an interrupt consistency check when adding a GPIO chip: if the chip is both gpio-controller and interrupt-controller, walk all children of the device tree, check if these in turn reference the interrupt-controller, and if they do, loop over the interrupts used by that child and perform gpio_reques() and gpio_direction_input() on these, making them unreachable from the GPIO side. Ugh, that's pretty awful, and it doesn't actually solve the root problem of the GPIO and IRQ subsystems not cooperating. It's also a very DT-centric solution even though we're going to see the exact same issue on ACPI machines. The problem is that the patches for OMAP that I applied and now have had to revert solves it in an even uglier way, leading to breaking boards, as was noticed. The approach in this patch has the potential to actually work without regressing a bunch of boards... Whether this is a problem in ACPI or not remains to be seen, but I'm not sure about that. Device trees allows for a GPIO line to be used as an interrupt source and GPIO line orthogonally, and that is the root of this problem. Does ACPI have the same problem, or does it impose natural restrictions on such use cases? I agree with Linus here. The problem is that GPIO controllers that can work as IRQ sources are treated in the kernel as if there where two separate controlers that are rather orthogonal: an irq_chip and a gpio_chip. But DT allows to use a GPIO line as an IRQ just by using an omap-gpio phandle as interrupt-parent. So, there should be a place where both irq_chip and gpio_chip has to be related somehow to properly configure a GPIO (request it and setting it as input) when used as an IRQ by DT. My patch for OMAP used an irq_domain_ops .map function handler to configure the GPIO when a IRQ was mapped since that seemed to me as the best place to do it. This worked well in OMAP2+ platforms but unfortunately broke OMAP1 platforms since they are still using legacy domain mapping thus not call .map. Linus' approach is much better IMHO since it is both generic and will only try to setup an GPIO when a GPIO bank is added by the DT core. So checking there if a GPIO line is being used as an IRQ and configuring the GPIO seems like a good solution to me. Old platforms like OMAP1 won't break since they don't (and probably never will) have DT support. We have to solve the problem in a better way than that. Rearranging your patch description, here are some of the points you brought up so I can comment on them... This has the following undesired effects: - The GPIOlib subsystem is not aware that the line is in use and willingly lets some other consumer perform gpio_request() on it, leading to a complex resource conflict if it occurs. If a gpio line is being both requested as a gpio and used as an interrupt line, then either a) it's a bug, or b) the gpio line needs to be used as input only so it is compatible with irq usage. b) should be supportable. Yes this is what I'm saying too I think... The bug in (a) manifested itself in the OMAP patch with no real solution in sight. - The GPIO debugfs file claims this GPIO line is free. Surely we can fix this. I still don't see a problem of having the controller request the gpio when it is claimed as an irq if we can get around the problem of another user performing a /valid/ request on the same GPIO line. The solution may be to have a special form of request or flag that allows it to be shared. I don't see how sharing works here, or how another user, i.e. another one than the user wanting to recieve the IRQ, can validly request such a line? What would the usecase for that valid request be? Basically I believe these two things need to be exclusive in the DT world: A: request_irq(a resource passed from interrupts); - core implicitly performs gpio_request() gpio_direction_input() B: gpio_request(a resource passed from gpios); gpio_direction_input() request_irq(gpio_to_irq()) Never both. And IIUC that was what happened in the OMAP case. Actually the OMAP case was not related to resource conflict but I'll explain the issues we had with the OMAP patch in a moment. First I just want to say that I don't think that makes sense that there are two users of a GPIO. There *could* be two users for the same IRQ that is mapped to a GPIO but in that case there will be only one user of the GPIO and that will be the IRQ controller even when in practice is the same chip and the same driver. If the same GPIO line is used for two devices in the DT, then gpio_request_one(gpio, GPIOF_IN, NULL) should be called just once and that state has to be saved somewhere so whoever setups the GPIO won't
Re: [PATCHv4 01/33] CLK: clkdev: add support for looking up clocks from DT
On 07/30/2013 06:04 PM, Nishanth Menon wrote:
On 07/23/2013 02:19 AM, Tero Kristo wrote:
clk_get_sys / clk_get can now find clocks from device-tree. If a DT clock
is found, an entry is added to the clk_lookup list also for subsequent
searches.
Signed-off-by: Tero Kristo t-kri...@ti.com
Cc: Russell King li...@arm.linux.org.uk
---
drivers/clk/clkdev.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 442a313..e39f082 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -93,6 +93,18 @@ struct clk *of_clk_get_by_name(struct device_node
*np, const char *name)
EXPORT_SYMBOL(of_clk_get_by_name);
#endif
+/**
+ * clkdev_add_nolock - add lookup entry for a clock
+ * @cl: pointer to new clock lookup entry
+ *
+ * Non-locking version, used internally by clk_find() to add DT based
+ * clock lookup entries.
+ */
+static void clkdev_add_nolock(struct clk_lookup *cl)
+{
+list_add_tail(cl-node, clocks);
+}
+
/*
* Find the correct struct clk for the device and connection ID.
* We do slightly fuzzy matching here:
@@ -106,6 +118,9 @@ static struct clk_lookup *clk_find(const char
*dev_id, const char *con_id)
{
struct clk_lookup *p, *cl = NULL;
int match, best_found = 0, best_possible = 0;
+struct device_node *node;
+struct clk *clk;
+struct of_phandle_args clkspec;
if (dev_id)
best_possible += 2;
@@ -133,6 +148,23 @@ static struct clk_lookup *clk_find(const char
*dev_id, const char *con_id)
break;
}
}
+
+if (cl)
+return cl;
+
+/* If clock was not found, attempt to look-up from DT */
+node = of_find_node_by_name(NULL, con_id);
+
+clkspec.np = node;
+
+clk = of_clk_get_from_provider(clkspec);
+
+if (!IS_ERR(clk)) {
+/* We found a clock, add node to clkdev */
+cl = clkdev_alloc(clk, con_id, dev_id);
clkdev_alloc may return NULL depending on vclkdev_alloc in which case
clkdev_add_nolock will crash trying to dereference it.
I'll add a check for that.
-Tero
+clkdev_add_nolock(cl);
+}
+
return cl;
}
--
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: [PATCHv4 02/33] clk: omap: introduce clock driver
On 07/30/2013 06:21 PM, Nishanth Menon wrote:
On 07/23/2013 02:19 AM, Tero Kristo wrote:
Parses OMAP clock data from DT and registers those clocks with the clock
framework. dt_omap_clk_init must be called early during boot for timer
initialization so it is exported and called from the existing clock code
instead of probing like a real driver. Based on initial work done by
Mike Turquette.
Signed-off-by: Tero Kristo t-kri...@ti.com
Cc: Mike Turquette mturque...@linaro.org
---
drivers/clk/Makefile |1 +
drivers/clk/omap/Makefile |1 +
drivers/clk/omap/clk.c| 39
+++
Minor suggestion - should we just start drivers/clk/ti/ instead of
clk/omap?
AM335x/DRA7 are not strictly OMAP.. might also allow for some reuse
for other TI platforms..
Not sure, this idea has been bounced around a bit. samsung has its own
directory under drivers/clk/ so maybe. I can change this if there is a
strong wish for this.
include/linux/clk/omap.h | 24
4 files changed, 65 insertions(+)
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c
create mode 100644 include/linux/clk/omap.h
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..d3c3733 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_ARCH_VT8500)+= clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ)+= zynq/
obj-$(CONFIG_ARCH_TEGRA)+= tegra/
obj-$(CONFIG_PLAT_SAMSUNG)+= samsung/
+obj-$(CONFIG_ARCH_OMAP)+= omap/
obj-$(CONFIG_X86)+= x86/
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
new file mode 100644
index 000..8195931
--- /dev/null
+++ b/drivers/clk/omap/Makefile
@@ -0,0 +1 @@
+obj-y+= clk.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
new file mode 100644
index 000..4bf1929
--- /dev/null
+++ b/drivers/clk/omap/clk.c
@@ -0,0 +1,39 @@
+/*
+ * OMAP PRCM clock driver
maybe then prcm-clk.c ?
Maybe remove the PRCM part. We have some clocks behind i2c for example,
and we might want to add support for them here.
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Tero Kristo t-kri...@ti.com
+ * Mike Turquette mturque...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/clk/omap.h
+#include linux/kernel.h
+#include linux/of_device.h
+
+/* FIXME - should the OMAP PRCM clock driver match generic types? */
should it? :)
Not 100% sure about the answer to this, the current implementation works
though so we could potentially drop this comment. :)
+static const struct of_device_id clk_match[] = {
+{.compatible = fixed-clock, .data = of_fixed_clk_setup, },
drivers/clk/clk-fixed-rate.c seems to already do this with
CLK_OF_DECLARE, or am I mistaken? and so on?
+{.compatible = mux-clock, .data = of_mux_clk_setup, },
+{.compatible = fixed-factor-clock,
+.data = of_fixed_factor_clk_setup, },
+{.compatible = divider-clock, .data = of_divider_clk_setup, },
+{.compatible = gate-clock, .data = of_gate_clk_setup, },
+{},
+};
+
+/* FIXME - need to initialize early; skip real driver registration
probe */
+int __init dt_omap_clk_init(void)
Might be good to have kernel doc to explain the init requirement as
documented in commit message.
Yeah, I am lacking docs once again.
+{
+of_clk_init(clk_match);
just doing of_clk_init(NULL); should do the job, no? that could even be
a static inline OR introduced in board_generic without additional headers?
I don't think that works, as we have special clock node types we want to
support (the TI specific types, and we want to override the default
behavior of some.)
+return 0;
+}
diff --git a/include/linux/clk/omap.h b/include/linux/clk/omap.h
new file mode 100644
index 000..647f02f
--- /dev/null
+++ b/include/linux/clk/omap.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
would you like to match licensing to that of the C file?
Ok, copy paste bug #2 for this. Initial version was worse than this though.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty
Re: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
HI,
On Wednesday 31 July 2013 01:19 PM, Felipe Balbi wrote:
Hi,
On Wed, Jul 31, 2013 at 11:17:52AM +0530, Sourav Poddar wrote:
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
new file mode 100644
index 000..3d10b69
--- /dev/null
+++ b/drivers/spi/spi-ti-qspi.c
@@ -0,0 +1,545 @@
snip
+/* Device Control */
+#define QSPI_DD(m, n) (m (3 + n*8))
+#define QSPI_CKPHA(n) (1 (2 + n*8))
+#define QSPI_CSPOL(n) (1 (1 + n*8))
+#define QSPI_CKPOL(n) (1 (n*8))
add spaces around the * operator
Ok.
+#defineQSPI_FRAME_MAX 0xfff
Frame max is 4096, 0x1000, right ?
Yes,
this macro was used initially to fill the register bits, where 4095 =
4096 words.
Will change it to now.
+static inline void ti_qspi_read_data(struct ti_qspi *qspi,
+ unsigned long reg, int wlen, u8 **rxbuf)
+{
+ switch (wlen) {
+ case 8:
+ **rxbuf = readb(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %02x\n, *(*rxbuf));
+ *rxbuf += 1;
+ break;
+ case 16:
+ **rxbuf = readw(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %04x\n, *(*rxbuf));
+ *rxbuf += 2;
+ break;
+ case 32:
+ **rxbuf = readl(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %04x\n, *(*rxbuf));
%08x, this was commented before.
Yes, My bad, will change.
+static int ti_qspi_setup(struct spi_device *spi)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi-master);
+ struct ti_qspi_regs *ctx_reg =qspi-ctx_reg;
+ int clk_div = 0, ret;
+ u32 clk_ctrl_reg, clk_rate, clk_mask;
+
+ clk_rate = clk_get_rate(qspi-fclk);
+
+ if (!qspi-spi_max_frequency) {
+ dev_err(qspi-dev, spi max frequency not defined\n);
+ return -EINVAL;
+ }
+
+ clk_div = DIV_ROUND_UP(clk_rate, qspi-spi_max_frequency) - 1;
+
+ dev_dbg(qspi-dev, %s: hz: %d, clock divider %d\n, __func__,
+ qspi-spi_max_frequency, clk_div);
+
+ ret = pm_runtime_get_sync(qspi-dev);
+ if (ret) {
+ dev_err(qspi-dev, pm_runtime_get_sync() failed\n);
+ return ret;
+ }
+
+ clk_ctrl_reg = ti_qspi_read(qspi, QSPI_SPI_CLOCK_CNTRL_REG);
+
+ clk_ctrl_reg= ~QSPI_CLK_EN;
+
+ if (spi-master-busy) {
+ dev_dbg(qspi-dev, master busy doing other trasnfers\n);
+ return -EBUSY;
+ }
this check can be done before pm_runtime_get_sync(), you're also leaking
pm_runtime reference here.
true. Will shift.
+ /* disable SCLK */
+ ti_qspi_write(qspi, clk_ctrl_reg, QSPI_SPI_CLOCK_CNTRL_REG);
+
+ if (clk_div 0) {
+ dev_dbg(qspi-dev, %s: clock divider 0, using /1 divider\n,
+ __func__);
+ pm_runtime_put_sync(qspi-dev);
+ return -EINVAL;
+ }
+
+ if (clk_div QSPI_CLK_DIV_MAX) {
+ dev_dbg(qspi-dev, %s: clock divider%d , using /%d divider\n,
+ __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1);
+ pm_runtime_put_sync(qspi-dev);
+ return -EINVAL;
+ }
why don't you move all checks to clk_div before pm_runtime_get_sync()
call ?
Make sense. Will move.
+static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ const u8 *txbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ txbuf = t-tx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
you're decrementing count by one, but in some cases you write 4 bytes or
2 bytes... This will blow up very soon. I can already see overflows
happening...
we write 2 bytes and 4 bytes for 16 bits_per_word and 32 bits_per_word case.
count is t-len, which is the total number of words to transfer. This
words can be of any length (1, 2 or 4) bytes. So, I think it should be
decremented by 1 only.
+static int qspi_read_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ u8 *rxbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ rxbuf = t-rx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
ditto
+static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ int ret;
+
+ if (t-tx_buf) {
+ ret = qspi_write_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi-dev, Error while writing\n);
+ return -EINVAL;
why do you change the return value from qspi_write_msg() ?
I was not sure about this, I thought I had signals an ETIMEOUT during
timeout, So signal a invalid transfer here.
Do you suggest keeping ETIMEOUT here also?
+ }
+ }
+
+ if (t-rx_buf) {
+ ret = qspi_read_msg(qspi, t);
+ if (ret) {
+
Re: [PATCH 4/9] dma: edma: Find missed events and issue them
On Wednesday 31 July 2013 10:19 AM, Joel Fernandes wrote: Hi Sekhar, On 07/30/2013 02:05 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: In an effort to move to using Scatter gather lists of any size with EDMA as discussed at [1] instead of placing limitations on the driver, we work through the limitations of the EDMAC hardware to find missed events and issue them. The sequence of events that require this are: For the scenario where MAX slots for an EDMA channel is 3: SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - Null The above SG list will have to be DMA'd in 2 sets: (1) SG1 - SG2 - SG3 - Null (2) SG4 - SG5 - SG6 - Null After (1) is succesfully transferred, the events from the MMC controller donot stop coming and are missed by the time we have setup the transfer for (2). So here, we catch the events missed as an error condition and issue them manually. Are you sure there wont be any effect of these missed events on the peripheral side. For example, wont McASP get into an underrun condition when it encounters a null PaRAM set? Even UART has to transmit to a But it will not encounter null PaRAM set because McASP uses contiguous buffers for transfer which are not scattered across physical memory. This can be accomplished with an SG of size 1. For such SGs, this patch series leaves it linked Dummy and does not link to Null set. Null set is only used for SG lists that are MAX_NR_SG in size such as those created for example by MMC and Crypto. particular baud so I guess it cannot wait like the way MMC/SD can. Existing driver have to wait anyway if they hit MAX SG limit today. If they don't want to wait, they would have allocated a contiguous block of memory and DMA that in one stretch so they don't lose any events, and in such cases we are not linking to Null. As long as DMA driver can advertize its MAX SG limit, peripherals can always work around that by limiting the number of sync events they generate so as to not having any of the events getting missed. With this series, I am worried that EDMA drivers is advertizing that it can handle any length SG list while not taking care of missing any events while doing so. This will break the assumptions that driver writers make. Also, wont this lead to under-utilization of the peripheral bandwith? Meaning, MMC/SD is ready with data but cannot transfer because the DMA is waiting to be set-up. But it is waiting anyway even today. Currently based on MAX segs, MMC driver/subsystem will make SG list of size max_segs. Between these sessions of creating such smaller SG-lists, if for some reason the MMC controller is sending events, these will be lost anyway. But if MMC/SD driver knows how many events it should generate if it knows the MAX SG limit. So there should not be any missed events in current code. And I am not claiming that your solution is making matters worse. But its not making it much better as well. What will happen now with this patch series is we are simply accepting a bigger list than this, and handling all the max_segs stuff within the EDMA driver itself without outside world knowing. This is actually more efficient as for long transfers, we are not going back and forth much between the client and EDMA driver. Agreed, I am not debating that we need to handle SG lists of any length. The hardware is capable of handling them, and no reason kernel should not. Did you consider a ping-pong scheme with say three PaRAM sets per channel? That way you can keep a continuous transfer going on from the peripheral over the complete SG list. Do you mean ping-pong scheme as used in the davinci-pcm driver today? No. AFAIR, thats a ping-pong between internal RAM and DDR for earlier audio ports which did not come with FIFO. This can be used only for buffers that are contiguous in memory, not those that are scattered across memory. I was hinting at using the linking facility of EDMA to achieve this. Each PaRAM set has full 32-bit source and destination pointers so I see no reason why non-contiguous case cannot be handled. Lets say you need to transfer SG[0..6] on channel C. Now, PaRAM sets are typically 4 times the number of channels. In this case we use one DMA PaRAM set and two Link PaRAM sets per channel. P0 is the DMA PaRAM set and P1 and P2 are the Link sets. Initial setup: SG0 - SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - NULL ^ ^ ^ | | | P0 - P1 - P2 - NULL P[0..2].TCINTEN = 1, so get an interrupt after each SG element completion. On each completion interrupt, hardware automatically copies the linked PaRAM set into the DMA PaRAM set so after SG0 is transferred out, the state of hardware is: SG1 - SG2 - SG3 - SG3 - SG6 - NULL ^ ^ | | P0,1P2 - NULL | ^ | | - SG1 transfer has already started by the time the TC interrupt is handled. As you can see P1 is now redundant and ready to be recycled. So in the
Re: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
Hi,
On Wed, Jul 31, 2013 at 02:40:51PM +0530, Sourav Poddar wrote:
+#defineQSPI_FRAME_MAX 0xfff
Frame max is 4096, 0x1000, right ?
Yes,
this macro was used initially to fill the register bits, where 4095 =
4096 words.
Will change it to now.
you can make this something like:
#define QSPI_FRAME(n) (((n) - 1) 0xfff)
+static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ const u8 *txbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ txbuf = t-tx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
you're decrementing count by one, but in some cases you write 4 bytes or
2 bytes... This will blow up very soon. I can already see overflows
happening...
we write 2 bytes and 4 bytes for 16 bits_per_word and 32 bits_per_word case.
count is t-len, which is the total number of words to transfer. This
t-len is total number of bytes as you can see from the documentation in
the header:
* @len: size of rx and tx buffers (in bytes)
As I said before, please read the documentation.
words can be of any length (1, 2 or 4) bytes. So, I think it should be
decremented by 1 only.
this is wrong.
+static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ int ret;
+
+ if (t-tx_buf) {
+ ret = qspi_write_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi-dev, Error while writing\n);
+ return -EINVAL;
why do you change the return value from qspi_write_msg() ?
I was not sure about this, I thought I had signals an ETIMEOUT during
timeout, So signal a invalid transfer here.
Do you suggest keeping ETIMEOUT here also?
yeah, so we tell whoever called us that the transfer timed out. If you
return -EINVAL you're telling your clients they gave you an invalid
spi_transfer, which is not the case.
+static int ti_qspi_start_transfer_one(struct spi_master *master,
+ struct spi_message *m)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(master);
+ struct spi_device *spi = m-spi;
+ struct spi_transfer *t;
+ int status = 0, ret;
+ int frame_length;
+
+ /* setup device control reg */
+ qspi-dc = 0;
+
+ if (spi-mode SPI_CPHA)
+ qspi-dc |= QSPI_CKPHA(spi-chip_select);
+ if (spi-mode SPI_CPOL)
+ qspi-dc |= QSPI_CKPOL(spi-chip_select);
+ if (spi-mode SPI_CS_HIGH)
+ qspi-dc |= QSPI_CSPOL(spi-chip_select);
+
+ frame_length = (m-frame_length 3) / spi-bits_per_word;
+
+ frame_length = clamp(frame_length, 0, QSPI_FRAME_MAX);
+
+ /* setup command reg */
+ qspi-cmd = 0;
+ qspi-cmd |= QSPI_EN_CS(spi-chip_select);
+ qspi-cmd |= QSPI_FLEN(frame_length);
+ qspi-cmd |= QSPI_WC_CMD_INT_EN;
+
+ ti_qspi_write(qspi, QSPI_WC_INT_EN, QSPI_INTR_ENABLE_SET_REG);
+
+ list_for_each_entry(t,m-transfers, transfer_list) {
no locking around list traversal ?
hmm..can put a spin_lock around qspi_transfer_msg ?
no dude, you need to protect the access to the list. So it needs to be
around list_for_each_entry().
+static irqreturn_t ti_qspi_isr(int irq, void *dev_id)
+{
+ struct ti_qspi *qspi = dev_id;
+ u16 stat;
+
+ irqreturn_t ret = IRQ_HANDLED;
+
+ spin_lock(qspi-lock);
+
+ stat = ti_qspi_read(qspi, QSPI_INTR_STATUS_ENABLED_CLEAR);
+
+ if (!stat) {
+ dev_dbg(qspi-dev, No IRQ triggered\n);
+ return IRQ_NONE;
leaving lock held.
Will add a unlock before returning.
there's a very nice C statement, goto, which you can use here.
+static irqreturn_t ti_qspi_threaded_isr(int this_irq, void *dev_id)
+{
+ struct ti_qspi *qspi = dev_id;
+ unsigned long flags;
+
+ spin_lock_irqsave(qspi-lock, flags);
+
+ complete(qspi-transfer_complete);
you need to check if your word completion is actually set. Don't assume
it's set because we might want to change the code later.
hmm..something like this.?
if (ti_qspi_read(qspi, QSPI_SPI_STATUS_REG) WC)
complete(qspi-transfer_complete);
I rather:
stat = ti_qspi_read(qspi, QSPI_SPI_STATUS_REG);
if (stat WC)
complete()
then, if we want to add frame interrupt handling later, we don't need to
read status register again. In fact, to avoid reading the status
register here, you could even cache the returned value you read in your
hardirq handler inside your qspi struct.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 7/9] ARM: edma: Don't clear EMR of channel in edma_stop
On Wednesday 31 July 2013 10:35 AM, Joel Fernandes wrote: On 07/30/2013 03:29 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: We certainly don't want error conditions to be cleared anywhere 'anywhere' is a really loaded term. as this will make us 'forget' about missed events. We depend on knowing which events were missed in order to be able to reissue them. This fixes a race condition where the EMR was being cleared by the transfer completion interrupt handler. Basically, what was happening was: Missed event | | V SG1-SG2-SG3-Null \ \__TC Interrupt (Almost same time as ARM is executing TC interrupt handler, an event got missed and also forgotten by clearing the EMR). Sorry, but I dont see how edma_stop() is coming into picture in the race you describe? In edma_callback function, for the case of DMA_COMPLETE (Transfer completion interrupt), edma_stop() is called when all sets have been processed. This had the effect of clearing the EMR. Ah, thanks. I was missing the fact that the race comes into picture only when using the DMA engine driver. I guess that should be mentioned somewhere since it is not immediately obvious. The patch looks good to me. So if you respin just this one with some updated explanation based on what you wrote below, I will take it. Thanks, Sekhar -- 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: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
On Wednesday 31 July 2013 02:50 PM, Felipe Balbi wrote:
Hi,
On Wed, Jul 31, 2013 at 02:40:51PM +0530, Sourav Poddar wrote:
+#defineQSPI_FRAME_MAX 0xfff
Frame max is 4096, 0x1000, right ?
Yes,
this macro was used initially to fill the register bits, where 4095 =
4096 words.
Will change it to now.
you can make this something like:
#define QSPI_FRAME(n) (((n) - 1) 0xfff)
Yes, but now its only used in a clamp function, where I should
provide the exact value 4096. Will use your previous suggestion.
#defineQSPI_FRAME_MAX0x1000
+static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ const u8 *txbuf;
+ int wlen, count, ret;
+
+ count = t-len;
+ txbuf = t-tx_buf;
+ wlen = t-bits_per_word;
+
+ while (count--) {
you're decrementing count by one, but in some cases you write 4 bytes or
2 bytes... This will blow up very soon. I can already see overflows
happening...
we write 2 bytes and 4 bytes for 16 bits_per_word and 32 bits_per_word case.
count is t-len, which is the total number of words to transfer. This
t-len is total number of bytes as you can see from the documentation in
the header:
* @len: size of rx and tx buffers (in bytes)
As I said before, please read the documentation.
words can be of any length (1, 2 or 4) bytes. So, I think it should be
decremented by 1 only.
this is wrong.
hmm..got the point.
I will pass the count address also to ti_qspi_read_data/write_data and make
use of the switch statement to decrement the count.
+static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
+{
+ int ret;
+
+ if (t-tx_buf) {
+ ret = qspi_write_msg(qspi, t);
+ if (ret) {
+ dev_dbg(qspi-dev, Error while writing\n);
+ return -EINVAL;
why do you change the return value from qspi_write_msg() ?
I was not sure about this, I thought I had signals an ETIMEOUT during
timeout, So signal a invalid transfer here.
Do you suggest keeping ETIMEOUT here also?
yeah, so we tell whoever called us that the transfer timed out. If you
return -EINVAL you're telling your clients they gave you an invalid
spi_transfer, which is not the case.
Ok.
+static int ti_qspi_start_transfer_one(struct spi_master *master,
+ struct spi_message *m)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(master);
+ struct spi_device *spi = m-spi;
+ struct spi_transfer *t;
+ int status = 0, ret;
+ int frame_length;
+
+ /* setup device control reg */
+ qspi-dc = 0;
+
+ if (spi-mode SPI_CPHA)
+ qspi-dc |= QSPI_CKPHA(spi-chip_select);
+ if (spi-mode SPI_CPOL)
+ qspi-dc |= QSPI_CKPOL(spi-chip_select);
+ if (spi-mode SPI_CS_HIGH)
+ qspi-dc |= QSPI_CSPOL(spi-chip_select);
+
+ frame_length = (m-frame_length 3) / spi-bits_per_word;
+
+ frame_length = clamp(frame_length, 0, QSPI_FRAME_MAX);
+
+ /* setup command reg */
+ qspi-cmd = 0;
+ qspi-cmd |= QSPI_EN_CS(spi-chip_select);
+ qspi-cmd |= QSPI_FLEN(frame_length);
+ qspi-cmd |= QSPI_WC_CMD_INT_EN;
+
+ ti_qspi_write(qspi, QSPI_WC_INT_EN, QSPI_INTR_ENABLE_SET_REG);
+
+ list_for_each_entry(t,m-transfers, transfer_list) {
no locking around list traversal ?
hmm..can put a spin_lock around qspi_transfer_msg ?
no dude, you need to protect the access to the list. So it needs to be
around list_for_each_entry().
Ok.
+static irqreturn_t ti_qspi_isr(int irq, void *dev_id)
+{
+ struct ti_qspi *qspi = dev_id;
+ u16 stat;
+
+ irqreturn_t ret = IRQ_HANDLED;
+
+ spin_lock(qspi-lock);
+
+ stat = ti_qspi_read(qspi, QSPI_INTR_STATUS_ENABLED_CLEAR);
+
+ if (!stat) {
+ dev_dbg(qspi-dev, No IRQ triggered\n);
+ return IRQ_NONE;
leaving lock held.
Will add a unlock before returning.
there's a very nice C statement, goto, which you can use here.
Ok.
+static irqreturn_t ti_qspi_threaded_isr(int this_irq, void *dev_id)
+{
+ struct ti_qspi *qspi = dev_id;
+ unsigned long flags;
+
+ spin_lock_irqsave(qspi-lock, flags);
+
+ complete(qspi-transfer_complete);
you need to check if your word completion is actually set. Don't assume
it's set because we might want to change the code later.
hmm..something like this.?
if (ti_qspi_read(qspi, QSPI_SPI_STATUS_REG) WC)
complete(qspi-transfer_complete);
I rather:
stat = ti_qspi_read(qspi, QSPI_SPI_STATUS_REG);
if (stat WC)
complete()
then, if we want to add frame interrupt handling later, we don't need to
read status register again. In fact, to avoid reading the status
register here, you could even cache the returned value you read in your
hardirq handler inside your qspi struct.
Ok.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a
Re: [PATCHv4 03/33] CLK: OMAP4: Add DPLL clock support
On 07/30/2013 07:23 PM, Nishanth Menon wrote:
This patch probably was submitted in the wrong sequence - fails build
and few other issues below.
Yeah, I'll double check the build sequence for these.
On 07/23/2013 02:19 AM, Tero Kristo wrote:
The OMAP clock driver now supports DPLL clock type. This patch also
adds support for DT DPLL nodes.
Then why is $subject specific to OMAP4? is that because of
of_omap4_dpll_setup?
The driver only supports omap4 dpll type at this point, omap3 dplls
require some modifications on top of this, and are provided later in the
series.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/Makefile |2 +-
drivers/clk/omap/clk.c|1 +
drivers/clk/omap/dpll.c | 295
+
Device Tree Binding documentation?
Didn't bother writing those yet as I haven't received too much feedback
whether this approach is acceptable or not.
3 files changed, 297 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/omap/dpll.c
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index 8195931..4cad480 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y+= clk.o
+obj-y+= clk.o dpll.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 4bf1929..1dafdaa 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -28,6 +28,7 @@ static const struct of_device_id clk_match[] = {
.data = of_fixed_factor_clk_setup, },
{.compatible = divider-clock, .data = of_divider_clk_setup, },
{.compatible = gate-clock, .data = of_gate_clk_setup, },
+{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
{},
};
you would not need this if you did just of_clk_init(NULL); ?
Why not? And I think we still need to do this.
Further, at this patch, build fails with:
drivers/clk/omap/clk.c:31:55: error: undefined identifier
'of_omap4_dpll_setup'
drivers/clk/omap/clk.c:31:48: error: ‘of_omap4_dpll_setup’ undeclared
here (not in a function)
which makes sense since we did not export the function.
Yea seems like wrong ordering of patches, sorry about that. .
diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
new file mode 100644
index 000..66e82be
--- /dev/null
+++ b/drivers/clk/omap/dpll.c
@@ -0,0 +1,295 @@
+/*
+ * OMAP DPLL clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h
after a quick check, are all these required?
Seems like some might not be needed, I'll double check this.
+#include linux/clk/omap.h
why?
Need dpll_data definition for example.
+
+static const struct clk_ops dpll_m4xen_ck_ops = {
+.enable= omap3_noncore_dpll_enable,
+.disable= omap3_noncore_dpll_disable,
+.recalc_rate= omap4_dpll_regm4xen_recalc,
+.round_rate= omap4_dpll_regm4xen_round_rate,
+.set_rate= omap3_noncore_dpll_set_rate,
+.get_parent= omap2_init_dpll_parent,
+};
+
+static const struct clk_ops dpll_core_ck_ops = {
+.recalc_rate= omap3_dpll_recalc,
+.get_parent= omap2_init_dpll_parent,
+};
+
+static const struct clk_ops dpll_ck_ops = {
+.enable= omap3_noncore_dpll_enable,
+.disable= omap3_noncore_dpll_disable,
+.recalc_rate= omap3_dpll_recalc,
+.round_rate= omap2_dpll_round_rate,
+.set_rate= omap3_noncore_dpll_set_rate,
+.get_parent= omap2_init_dpll_parent,
+.init= omap2_init_clk_clkdm,
+};
+
+static const struct clk_ops dpll_x2_ck_ops = {
+.recalc_rate= omap3_clkoutx2_recalc,
+};
none of these are defined at this stage of the patch, generates a huge
build error for dpll.c
http://pastebin.com/GJucv1A5
Yea, wrong ordering, linux/clk/omap.h is not up to date. I'll fix this
and rest of the similar issues.
+
+struct clk *omap_clk_register_dpll(struct device *dev, const char *name,
+const char **parent_names, int num_parents, unsigned long flags,
+struct dpll_data *dpll_data, const char *clkdm_name,
+const struct clk_ops *ops)
why should this be public?
Currently does not need to be, but someone could in theory build up
cclockXxxx_data.c file and use these calls
Re: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
Hi, On Wed, Jul 31, 2013 at 03:10:40PM +0530, Sourav Poddar wrote: words can be of any length (1, 2 or 4) bytes. So, I think it should be decremented by 1 only. this is wrong. hmm..got the point. I will pass the count address also to ti_qspi_read_data/write_data and make use of the switch statement to decrement the count. why don't you return the amount of bytes to decrement in case of success ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 1/3] Input: omap-keypad: Enable wakeup capability for keypad.
On Mon, Jul 29, 2013 at 01:40:28PM -0700, Dmitry Torokhov wrote:
On Monday, July 29, 2013 11:36:05 PM Felipe Balbi wrote:
Hi,
On Mon, Jul 29, 2013 at 12:59:23PM -0700, Dmitry Torokhov wrote:
@@ -439,12 +444,50 @@ static const struct of_device_id
omap_keypad_dt_match[] = {
MODULE_DEVICE_TABLE(of, omap_keypad_dt_match);
#endif
+#ifdef CONFIG_PM_SLEEP
+static int omap4_keypad_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
you don't need to access the platform_device...
+ struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
... since this can become:
struct omap4_keypad *keypad_data = dev_get_drvdata(dev);
No, please use correct accessors for the objects. Platform drivers
deal
with platform devices and I prefer using platform_get_drvdata() on
them.
The argument to this function is a struct device, you prefer to do some
pointer math to find the containing pdev, then deref that back to dev,
then to struct device_private and further to driver_data ?
Sounds like a waste of time IMHO. You already have the device pointer
anyway, why would you go through the trouble of calculating the
offsets for the containing struct platform_device ?
This assumes knowledge of dev_get_drvdata() implementation and assumption
that it will stay the same. Unless I hear from device core guys that
bus_{get|set}_drvdata() methods are obsolete and will be eventually
removed I will require proper accessors being used.
they're not obsolete and will never be removed. They're nothing but
helpers though. Instead of calling:
dev_set_drvdata(pdev-dev);
you call:
platform_set_drvdata(pdev);
same is valid for every single bus, but in the end they all just wrap a
call dev_{set,get}_drvdata() internally. If you already have a struct
device pointer as argument, why waste cycles doing pointer math just to
go back to the same struct device pointer on the next call ?
Because I do not want to rely on the fact that what my driver set up
with platform_set_drvdata(pdev, XXX) is the same as what dev_get_drvdata()
will return *in the current implementation*. Software layers and all
that...
fair enough, your call. It's a waste of CPU anyway.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
On Wednesday 31 July 2013 03:18 PM, Felipe Balbi wrote: Hi, On Wed, Jul 31, 2013 at 03:10:40PM +0530, Sourav Poddar wrote: words can be of any length (1, 2 or 4) bytes. So, I think it should be decremented by 1 only. this is wrong. hmm..got the point. I will pass the count address also to ti_qspi_read_data/write_data and make use of the switch statement to decrement the count. why don't you return the amount of bytes to decrement in case of success ? Yes, that can be done to tackle this. Thanks! -- 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: [PATCHv4 04/33] CLK: omap: move part of the machine specific clock header contents to driver
On 07/30/2013 09:22 PM, Nishanth Menon wrote:
this patch should be 3/33 to allow dpll.c to build.
On 07/23/2013 02:19 AM, Tero Kristo wrote:
Some of the clock.h contents are needed by the new OMAP clock driver,
including dpll_data and clk_hw_omap. Thus, move these to the generic
omap header file which can be accessed by the driver.
Looking at the change, I wonder what the rules are for the movement?
commit message was not clear.
This is kind of a merge of almost everything that is needed by clock
drivers at some point. I can move the changes along to the patches that
actually need the exports for clarity and to keep compilation clean.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
arch/arm/mach-omap2/clock.h | 151
+
include/linux/clk/omap.h| 157
++-
2 files changed, 157 insertions(+), 151 deletions(-)
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index 7aa32cd..d1a3125 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -21,6 +21,7 @@
#include linux/clkdev.h
#include linux/clk-provider.h
+#include linux/clk/omap.h
struct omap_clk {
u16cpu;
@@ -178,83 +179,6 @@ struct clksel {
const struct clksel_rate *rates;
};
-/**
- * struct dpll_data - DPLL registers and integration data
- * @mult_div1_reg: register containing the DPLL M and N bitfields
- * @mult_mask: mask of the DPLL M bitfield in @mult_div1_reg
- * @div1_mask: mask of the DPLL N bitfield in @mult_div1_reg
- * @clk_bypass: struct clk pointer to the clock's bypass clock input
- * @clk_ref: struct clk pointer to the clock's reference clock input
- * @control_reg: register containing the DPLL mode bitfield
- * @enable_mask: mask of the DPLL mode bitfield in @control_reg
- * @last_rounded_rate: cache of the last rate result of
omap2_dpll_round_rate()
- * @last_rounded_m: cache of the last M result of
omap2_dpll_round_rate()
- * @last_rounded_m4xen: cache of the last M4X result of
- *omap4_dpll_regm4xen_round_rate()
- * @last_rounded_lpmode: cache of the last lpmode result of
- * omap4_dpll_lpmode_recalc()
- * @max_multiplier: maximum valid non-bypass multiplier value (actual)
- * @last_rounded_n: cache of the last N result of
omap2_dpll_round_rate()
- * @min_divider: minimum valid non-bypass divider value (actual)
- * @max_divider: maximum valid non-bypass divider value (actual)
- * @modes: possible values of @enable_mask
- * @autoidle_reg: register containing the DPLL autoidle mode bitfield
- * @idlest_reg: register containing the DPLL idle status bitfield
- * @autoidle_mask: mask of the DPLL autoidle mode bitfield in
@autoidle_reg
- * @freqsel_mask: mask of the DPLL jitter correction bitfield in
@control_reg
- * @idlest_mask: mask of the DPLL idle status bitfield in @idlest_reg
- * @lpmode_mask: mask of the DPLL low-power mode bitfield in
@control_reg
- * @m4xen_mask: mask of the DPLL M4X multiplier bitfield in @control_reg
- * @auto_recal_bit: bitshift of the driftguard enable bit in
@control_reg
- * @recal_en_bit: bitshift of the PRM_IRQENABLE_* bit for
recalibration IRQs
- * @recal_st_bit: bitshift of the PRM_IRQSTATUS_* bit for
recalibration IRQs
- * @flags: DPLL type/features (see below)
- *
- * Possible values for @flags:
- * DPLL_J_TYPE: J-type DPLL (only some 36xx, 4xxx DPLLs)
- *
- * @freqsel_mask is only used on the OMAP34xx family and AM35xx.
- *
- * XXX Some DPLLs have multiple bypass inputs, so it's not technically
- * correct to only have one @clk_bypass pointer.
- *
- * XXX The runtime-variable fields (@last_rounded_rate, @last_rounded_m,
- * @last_rounded_n) should be separated from the runtime-fixed fields
- * and placed into a different structure, so that the runtime-fixed data
- * can be placed into read-only space.
- */
-struct dpll_data {
-void __iomem*mult_div1_reg;
-u32mult_mask;
-u32div1_mask;
-struct clk*clk_bypass;
-struct clk*clk_ref;
-void __iomem*control_reg;
-u32enable_mask;
-unsigned longlast_rounded_rate;
-u16last_rounded_m;
-u8last_rounded_m4xen;
-u8last_rounded_lpmode;
-u16max_multiplier;
-u8last_rounded_n;
-u8min_divider;
-u16max_divider;
-u8modes;
-void __iomem*autoidle_reg;
-void __iomem*idlest_reg;
-u32autoidle_mask;
-u32freqsel_mask;
-u32idlest_mask;
-u32dco_mask;
-u32sddiv_mask;
-u32lpmode_mask;
-u32m4xen_mask;
-u8auto_recal_bit;
-u8recal_en_bit;
-u8recal_st_bit;
-u8flags;
-};
-
/*
* struct clk.flags possibilities
*
@@ -274,56 +198,6 @@ struct dpll_data {
Re: [PATCHv4 05/33] CLK: omap: add DT duplicate clock registration mechanism
On 07/30/2013 09:40 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
Some devices require their clocks to be available with a specific
dev-id con-id mapping. With DT, the clocks can be found by default
only with their name, or alternatively through the device node of
the consumer. With drivers, that don't support DT fully yet, add
mechanism to register specific clock names.
Signed-off-by: Tero Kristo t-kri...@ti.com
with this, should it not be enough to add clocks=phandle
Don't know, I am not an expert on DT. :)
I am not sure I understand what specific drivers should need to carry
this old hack forward. More importantly, why is it preferable to carry
the hack forward rather than fixing the drivers.
At least the GP timer seems to need this, and I don't want to touch /
verify all the potential drivers touched by this so it is easier to
provide a (semi) temporary tweak.
---
drivers/clk/omap/clk.c | 39 +++
include/linux/clk/omap.h | 17 +
2 files changed, 56 insertions(+)
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 1dafdaa..cd31a81 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -32,6 +32,45 @@ static const struct of_device_id clk_match[] = {
{},
};
+ /**
+ * omap_dt_clocks_register - register DT duplicate clocks during boot
+ * @oclks: list of clocks to register
+ * @cnt: number of clocks
+ *
+ * Register duplicate or non-standard DT clock entries during boot. By
+ * default, DT clocks are found based on their node name. If any
+ * additional con-id / dev-id - clock mapping is required, use this
+ * function to list these.
+ */
+void __init omap_dt_clocks_register(struct omap_dt_clk oclks[], int cnt)
Cant we use a NULL terminated array? then we dont need to pass cnt.
Yea can.
+{
+struct omap_dt_clk *c;
+struct device_node *n;
+struct clk *clk;
+struct of_phandle_args clkspec;
+
+for (c = oclks; c oclks + cnt; c++) {
+n = of_find_node_by_name(NULL, c-node_name);
+
+if (!n) {
+pr_err(%s: %s not found!\n, __func__, c-node_name);
+continue;
+}
+
+clkspec.np = n;
+
+clk = of_clk_get_from_provider(clkspec);
+
+if (!clk) {
+pr_err(%s: %s has no clock!\n, __func__,
+c-node_name);
+continue;
+}
+c-lk.clk = clk;
+clkdev_add(c-lk);
why not clk_add_alias ?
Hmm yea, that might work also now that I made patch #1.
+}
+}
+
/* FIXME - need to initialize early; skip real driver registration
probe */
int __init dt_omap_clk_init(void)
{
diff --git a/include/linux/clk/omap.h b/include/linux/clk/omap.h
index cba892a..c39e775 100644
--- a/include/linux/clk/omap.h
+++ b/include/linux/clk/omap.h
@@ -19,6 +19,8 @@
#ifndef __LINUX_CLK_OMAP_H_
#define __LINUX_CLK_OMAP_H_
+#include linux/clkdev.h
+
/**
* struct dpll_data - DPLL registers and integration data
* @mult_div1_reg: register containing the DPLL M and N bitfields
@@ -146,6 +148,20 @@ struct clk_hw_omap_ops {
void(*deny_idle)(struct clk_hw_omap *oclk);
};
+struct omap_dt_clk {
+struct clk_lookuplk;
+const char*node_name;
+};
+
documentation missing.
Yea, will add.
+#define DT_CLK(dev, con, name)\
+{\
+.lk = {\
+.dev_id = dev,\
+.con_id = con,\
+},\
+.node_name = name,\
+}
+
void omap2_init_clk_hw_omap_clocks(struct clk *clk);
int omap3_noncore_dpll_enable(struct clk_hw *hw);
void omap3_noncore_dpll_disable(struct clk_hw *hw);
@@ -174,6 +190,7 @@ extern const struct clk_hw_omap_ops
clkhwops_omap4_dpllmx;
/* DT functions */
int dt_omap_clk_init(void);
+extern void omap_dt_clocks_register(struct omap_dt_clk *oclks, int cnt);
do you need the extern?
I guess not.
void of_omap4_dpll_setup(struct device_node *node);
#endif
--
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: [PATCHv4 06/33] CLK: omap: add autoidle support
On 07/30/2013 09:56 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
OMAP clk driver now routes some of the basic clocks through own
registration routine to allow autoidle support. This routine just
checks a couple of device node properties and adds autoidle support
if required, and just passes the registration forward to basic clocks.
why not extend standard framework to support autoidle capable clocks OR
introduce our own clk node which depends on basic clocks?
Was kind of easier this way.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
arch/arm/mach-omap2/clock.c |6 ++
drivers/clk/omap/Makefile |2 +-
drivers/clk/omap/autoidle.c | 130
+++
drivers/clk/omap/clk.c |4 +-
include/linux/clk/omap.h|4 ++
5 files changed, 143 insertions(+), 3 deletions(-)
create mode 100644 drivers/clk/omap/autoidle.c
I know it is getting a little stale, but anyways, device tree binding
missing.
diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 0c38ca9..669d4c4 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
Not sure why, at this point in time we are going to calling drivers/clk
code.
@@ -520,6 +520,9 @@ int omap2_clk_enable_autoidle_all(void)
list_for_each_entry(c, clk_hw_omap_clocks, node)
if (c-ops c-ops-allow_idle)
c-ops-allow_idle(c);
+
+of_omap_clk_allow_autoidle_all();
+
return 0;
}
@@ -539,6 +542,9 @@ int omap2_clk_disable_autoidle_all(void)
list_for_each_entry(c, clk_hw_omap_clocks, node)
if (c-ops c-ops-deny_idle)
c-ops-deny_idle(c);
+
+of_omap_clk_deny_autoidle_all();
+
these are defined for CONFIG_OF.. anyways, without dt nodes (OMAP3 is
supposed to support non-DT boot even now), this would not work, would it?
The lists are empty so the funcs do nothing. However, dropping CONFIG_OF
would break these of course. Will figure out a fix for this.
The calls are needed for the transition phase until we can move more clk
stuff from mach-omap2 to drivers.
return 0;
}
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index 4cad480..ca56700 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y+= clk.o dpll.o
+obj-y+= clk.o dpll.o autoidle.o
diff --git a/drivers/clk/omap/autoidle.c b/drivers/clk/omap/autoidle.c
new file mode 100644
index 000..6424cb2
--- /dev/null
+++ b/drivers/clk/omap/autoidle.c
@@ -0,0 +1,130 @@
+/*
+ * OMAP clock autoidle support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h
at all of these required?
I'll double check.
+
+#ifdef CONFIG_OF
+struct clk_omap_autoidle {
+void __iomem*reg;
+u8shift;
+u8flags;
+const char*name;
+struct list_headnode;
+};
+
+#define AUTOIDLE_LOW0x1
+
+static LIST_HEAD(autoidle_clks);
+
+static void omap_allow_autoidle(struct clk_omap_autoidle *clk)
+{
+u32 val;
+
+val = readl(clk-reg);
+
+if (clk-flags AUTOIDLE_LOW)
+val = ~(1 clk-shift);
+else
+val |= (1 clk-shift);
+
+writel(val, clk-reg);
+}
+
+static void omap_deny_autoidle(struct clk_omap_autoidle *clk)
+{
+u32 val;
+
+val = readl(clk-reg);
+
+if (clk-flags AUTOIDLE_LOW)
+val |= (1 clk-shift);
+else
+val = ~(1 clk-shift);
+
+writel(val, clk-reg);
+}
+
+void of_omap_clk_allow_autoidle_all(void)
+{
+struct clk_omap_autoidle *c;
+
+list_for_each_entry(c, autoidle_clks, node)
+omap_allow_autoidle(c);
+}
+
+void of_omap_clk_deny_autoidle_all(void)
+{
+struct clk_omap_autoidle *c;
+
+list_for_each_entry(c, autoidle_clks, node)
+omap_deny_autoidle(c);
+}
+
+static __init void of_omap_autoidle_setup(struct device_node *node)
+{
+u32 shift;
+void __iomem *reg;
+struct clk_omap_autoidle *clk;
+
+if (of_property_read_u32(node, ti,autoidle-shift, shift))
+return;
+
+reg = of_iomap(node, 0);
+
+clk = kzalloc(sizeof(struct clk_omap_autoidle), GFP_KERNEL);
+
+if (!clk) {
+pr_err(%s: kzalloc failed\n, __func__);
+return;
+}
+
+
Re: [PATCH v3] Documentation: dt: bindings: TI WiLink modules
Hi Luciano,
On Tuesday 30 July 2013 23:21:08 Luciano Coelho wrote:
Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.
Signed-off-by: Luciano Coelho coe...@ti.com
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
In v3, use IRQ_TYPE_LEVEL_HIGH in the example, as suggested by Laurent.
.../devicetree/bindings/net/wireless/ti-wilink.txt | 68
++ 1 file changed, 68 insertions(+)
create mode 100644
Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt new file
mode 100644
index 000..aafebb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,68 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8. WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+ See the interrupt controller's bindings documentation for
+ detailed definition.
+
+Optional properties:
+
+
+- clocks: list of clocks needed by the chip as follows:
+
+ refclock: the internal WLAN reference clock frequency (required for
+ WiLink6 and WiLink7; not used for WiLink8).
+
+ tcxoclock: the internal WLAN TCXO clock frequency (required for
+ WiLink7 not used for WiLink6 and WiLink8).
+
+ The clocks must be defined and named accordingly. For example:
+
+ clocks = refclock
+ clock-names = refclock;
+
+ refclock: refclock {
+ compatible = ti,wilink-clock;
+ #clock-cells = 0;
+ clock-frequency = 3840;
+ };
+
+ Some modules that contain the WiLink chip provide clocks in the
+ module itself. In this case, we define a ti,wilink-clock as shown
+ above. But any other clock could in theory be used, so the proper
+ clock definition should be used.
+
+
+Example:
+
+
+Example definition that can be used in OMAP4 Panda:
+
+wlan {
+ compatible = ti,wilink6;
+ interrupt-parent = gpio2;
+ interrupts = 21 IRQ_TYPE_LEVEL_HIGH; /* gpio line 53 */
+ clocks = refclock;
+ clock-names = refclock;
+
+ refclock: refclock {
+ compatible = ti,wilink-clock;
+ #clock-cells = 0;
+ clock-frequency = 3840;
+ };
+};
--
Regards,
Laurent Pinchart
--
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: Linux kernel for OMAP5432 uEVM
On 07/31/2013 11:31 AM, Lokesh Vutla wrote: Hi Chen, On Wednesday 31 July 2013 01:19 PM, Chen Baozi wrote: On Jul 30, 2013, at 11:52 AM, Lokesh Vutla lokeshvu...@ti.com wrote: You can also use Linus's kernel with the above clk data branch.( OMAP5 uEVM boots) Please let me know if you need more info. And the ethernet driver is not available by default? You need the following patch series recently posted by Roger: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92694.html Those will work only if you proceed with Tero's clock data in DT approach. cheers, -roger -- 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
[net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
The new IP version has a minor changes and the offsets are same as the previous
version, so instead of adding CPSW version number in the driver, make the driver
to fall through to the latest versions so that the new version of CPSW which has
the same register offsets will work directly without patching the driver.
Signed-off-by: Mugunthan V N mugunthan...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
drivers/net/ethernet/ti/cpsw.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 05a1674..a6b9700 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -799,6 +799,7 @@ static void cpsw_slave_open(struct cpsw_slave *slave,
struct cpsw_priv *priv)
slave_write(slave, TX_PRIORITY_MAPPING, CPSW1_TX_PRI_MAP);
break;
case CPSW_VERSION_2:
+ default:
slave_write(slave, TX_PRIORITY_MAPPING, CPSW2_TX_PRI_MAP);
break;
}
@@ -1180,10 +1181,9 @@ static int cpsw_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
cpsw_hwtstamp_v1(priv);
break;
case CPSW_VERSION_2:
+ default:
cpsw_hwtstamp_v2(priv);
break;
- default:
- return -ENOTSUPP;
}
return copy_to_user(ifr-ifr_data, cfg, sizeof(cfg)) ? -EFAULT : 0;
@@ -1790,6 +1790,7 @@ static int cpsw_probe(struct platform_device *pdev)
dma_params.desc_mem_phys = 0;
break;
case CPSW_VERSION_2:
+ default:
priv-host_port_regs = ss_regs + CPSW2_HOST_PORT_OFFSET;
priv-cpts-reg = ss_regs + CPSW2_CPTS_OFFSET;
dma_params.dmaregs = ss_regs + CPSW2_CPDMA_OFFSET;
@@ -1801,10 +1802,6 @@ static int cpsw_probe(struct platform_device *pdev)
dma_params.desc_mem_phys =
(u32 __force) priv-cpsw_res-start + CPSW2_BD_OFFSET;
break;
- default:
- dev_err(priv-dev, unknown version 0x%08x\n, priv-version);
- ret = -ENODEV;
- goto clean_cpsw_wr_iores_ret;
}
for (i = 0; i priv-data.slaves; i++) {
struct cpsw_slave *slave = priv-slaves[i];
--
1.8.4.rc0.11.g35f5eaa
--
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: Linux kernel for OMAP5432 uEVM
Hi Chen, On 07/31/2013 02:31 PM, Roger Quadros wrote: On 07/31/2013 11:31 AM, Lokesh Vutla wrote: Hi Chen, On Wednesday 31 July 2013 01:19 PM, Chen Baozi wrote: On Jul 30, 2013, at 11:52 AM, Lokesh Vutla lokeshvu...@ti.com wrote: You can also use Linus's kernel with the above clk data branch.( OMAP5 uEVM boots) Please let me know if you need more info. And the ethernet driver is not available by default? You need the following patch series recently posted by Roger: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92694.html Those will work only if you proceed with Tero's clock data in DT approach. I've created a branch that contains all patches required to get ethernet working on uEVM on top of Tero's DT clocks. I took the leisure of picking the baseport tree from Lokesh [1]. You can get the necessary stuff from branch usbhost-uevm-3.11-rc3 in git tree git://github.com/rogerq/linux.git cheers, -roger [1] - https://github.com/lokeshvutla/linux/tree/dra7-3.11-rc3-base -- 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: Linux kernel for OMAP5432 uEVM
Hi Roger, On Jul 31, 2013, at 8:16 PM, Roger Quadros rog...@ti.com wrote: Hi Chen, On 07/31/2013 02:31 PM, Roger Quadros wrote: On 07/31/2013 11:31 AM, Lokesh Vutla wrote: Hi Chen, On Wednesday 31 July 2013 01:19 PM, Chen Baozi wrote: On Jul 30, 2013, at 11:52 AM, Lokesh Vutla lokeshvu...@ti.com wrote: You can also use Linus's kernel with the above clk data branch.( OMAP5 uEVM boots) Please let me know if you need more info. And the ethernet driver is not available by default? You need the following patch series recently posted by Roger: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg92694.html Those will work only if you proceed with Tero's clock data in DT approach. I've created a branch that contains all patches required to get ethernet working on uEVM on top of Tero's DT clocks. I took the leisure of picking the baseport tree from Lokesh [1]. You can get the necessary stuff from branch usbhost-uevm-3.11-rc3 in git tree git://github.com/rogerq/linux.git Thanks a lot. I'll have a try. Baozi -- 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/4] ARM: dts: omap4-panda: add MMC5 (WiLink WLAN) configuration
On Wednesday 31 July 2013 02:45 AM, Luciano Coelho wrote:
Add regulator, pin muxing and MMC5 configuration to be used by the
on-board WiLink6 module.
Signed-off-by: Luciano Coelho coe...@ti.com
---
arch/arm/boot/dts/omap4-panda-common.dtsi | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi
b/arch/arm/boot/dts/omap4-panda-common.dtsi
index faa95b5..b3f6e1f 100644
--- a/arch/arm/boot/dts/omap4-panda-common.dtsi
+++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
@@ -107,6 +107,16 @@
*/
clock-frequency = 1920;
};
+
+ wilink_wl_en: fixedregulator@1 {
+ compatible = regulator-fixed;
+ regulator-name = wilink_wl_en;
+ regulator-min-microvolt = 180;
+ regulator-max-microvolt = 180;
+ gpio = gpio2 11 0; /* gpio line 43 */
+ startup-delay-us = 7;
+ enable-active-high;
+ };
};
omap4_pmx_wkup {
@@ -132,6 +142,7 @@
dss_hdmi_pins
tpd12s015_pins
hsusbb1_pins
+ wilink_pins
;
twl6030_pins: pinmux_twl6030_pins {
@@ -235,6 +246,19 @@
0x1c (PIN_OUTPUT | MUX_MODE3) /* gpio_wk8 */
;
};
+
+ wilink_pins: pinmux_wilink_pins {
+ pinctrl-single,pins =
+ 0x7a 0x103 /* gpio_53 INPUT | MODE3 */
+ 0x66 0x3/* gpio_43 OUTPUT | MODE3 */
+ 0x148 0x118 /* clk INPUT PULLUP | MODE0 */
+ 0x14a 0x118 /* cmd INPUT PULLUP | MODE0 */
+ 0x14c 0x118 /* dat0 INPUT PULLUP | MODE0 */
+ 0x14e 0x118 /* dat1 INPUT PULLUP | MODE0 */
+ 0x150 0x118 /* dat2 INPUT PULLUP | MODE0 */
+ 0x152 0x118 /* dat3 INPUT PULLUP | MODE0 */
Hi,
Since the base for omap4_pmx_core is 0x4a100040, you need to offset 0x40 from
pad address :-)
and can you please use INPUT_EN / PIN_INPUT_PULLUP / MUX_MODEx macros
(from dt-bindings/pinctrl/omap.h)
--
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: [PATCHv4 07/33] CLK: omap: add support for OMAP gate clock
On 07/30/2013 10:17 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
This node adds support for a clock node which allows control to the
clockdomain enable / disable.
Dont we have clkdm_enable/disable for the same? should we model
clockdomain as a clock node?
There was some discussion about having clockdomain code under
drivers/clk while back, but Mike turned this idea down.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/Makefile |2 +-
drivers/clk/omap/clk.c|1 +
drivers/clk/omap/gate.c | 88
+
include/linux/clk/omap.h |1 +
4 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/omap/gate.c
my usual crib: device tree binding documentation is missing
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index ca56700..3d3ca30f 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1 +1 @@
-obj-y+= clk.o dpll.o autoidle.o
+obj-y+= clk.o dpll.o autoidle.o gate.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index c149097..8c89714 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -29,6 +29,7 @@ static const struct of_device_id clk_match[] = {
{.compatible = divider-clock, .data = of_omap_divider_setup, },
{.compatible = gate-clock, .data = of_gate_clk_setup, },
{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
+{.compatible = ti,gate-clock, .data = of_omap_gate_clk_setup, },
I am a little lost - is there any SoC dts that actually uses this? at
least this series does not seem to introduce any node that uses this
compatibility as per git grep :(
There is, see patch 08/33 or 28/33.
might as well drop the patch?
{},
};
diff --git a/drivers/clk/omap/gate.c b/drivers/clk/omap/gate.c
new file mode 100644
index 000..7186bb2
--- /dev/null
+++ b/drivers/clk/omap/gate.c
@@ -0,0 +1,88 @@
+/*
+ * OMAP gate clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/clk/omap.h
+
+#ifdef CONFIG_OF
+
+static const struct clk_ops omap_gate_clk_ops = {
+.init= omap2_init_clk_clkdm,
+.enable= omap2_clkops_enable_clkdm,
+.disable= omap2_clkops_disable_clkdm,
+};
+
+void __init of_omap_gate_clk_setup(struct device_node *node)
+{
+struct clk *clk;
+struct clk_init_data init;
init = { 0 };
Will add.
+struct clk_hw_omap *clk_hw;
+const char *clk_name = node-name;
+int num_parents;
+const char **parent_names;
+int i;
+
+clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);
kzalloc(sizeof(*clk_hw)...) over kzalloc(sizeof(struct clk_hw_omap)...)
+if (!clk_hw) {
+pr_err(%s: could not allocate clk_hw_omap\n, __func__);
+return;
+}
+
+clk_hw-hw.init = init;
+
+of_property_read_string(node, clock-output-names, clk_name);
+of_property_read_string(node, ti,clkdm-name, clk_hw-clkdm_name);
+
+init.name = clk_name;
+init.ops = omap_gate_clk_ops;
+
+num_parents = of_clk_get_parent_count(node);
+if (num_parents 1) {
+pr_err(%s: omap trace_clk %s must have parent(s)\n,
+__func__, node-name);
CHECK: Alignment should match open parenthesis
I still wonder which version of checkpatch you are using.
+goto cleanup;
+}
+
+parent_names = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+
+for (i = 0; i num_parents; i++)
+parent_names[i] = of_clk_get_parent_name(node, i);
+
+init.num_parents = num_parents;
+init.parent_names = parent_names;
+
+clk = clk_register(NULL, clk_hw-hw);
+
+if (!IS_ERR(clk)) {
+of_clk_add_provider(node, of_clk_src_simple_get, clk);
+return;
+}
+
+cleanup:
kfree(parent_names)?
Yea, will add.
+kfree(clk_hw);
+}
+EXPORT_SYMBOL(of_omap_gate_clk_setup);
+CLK_OF_DECLARE(omap_gate_clk, ti,omap-gate-clock,
of_omap_gate_clk_setup);
+#endif
diff --git a/include/linux/clk/omap.h b/include/linux/clk/omap.h
index 904bdad..58ebb80 100644
--- a/include/linux/clk/omap.h
+++ b/include/linux/clk/omap.h
@@ -194,6 +194,7 @@ extern void omap_dt_clocks_register(struct
omap_dt_clk
Re: [PATCHv4 08/33] ARM: dts: omap4 clock data
On 07/30/2013 10:27 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
This patch creates a unique node for each clock in the OMAP4 power,
reset and clock manager (PRCM). OMAP443x and OMAP446x have slightly
different clock tree which is taken into account in the data.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
arch/arm/boot/dts/omap443x-clocks.dtsi | 17 +
arch/arm/boot/dts/omap443x.dtsi|8 +
arch/arm/boot/dts/omap4460.dtsi|8 +
arch/arm/boot/dts/omap446x-clocks.dtsi | 27 +
arch/arm/boot/dts/omap44xx-clocks.dtsi | 1654
arch/arm/boot/dts/omap44xx-common-clocks.dtsi ?
5 files changed, 1714 insertions(+)
create mode 100644 arch/arm/boot/dts/omap443x-clocks.dtsi
create mode 100644 arch/arm/boot/dts/omap446x-clocks.dtsi
create mode 100644 arch/arm/boot/dts/omap44xx-clocks.dtsi
diff --git a/arch/arm/boot/dts/omap443x-clocks.dtsi
b/arch/arm/boot/dts/omap443x-clocks.dtsi
new file mode 100644
index 000..2bd82b2
--- /dev/null
+++ b/arch/arm/boot/dts/omap443x-clocks.dtsi
@@ -0,0 +1,17 @@
+/*
+ * Device Tree Source for OMAP443x clock data
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
Doing
/include/ omap44xx-clocks.dtsi might avoid including that header in
corresponding SoC dtsi,
OR:
+bandgap_fclk: bandgap_fclk@4a307888 {
+#clock-cells = 0;
+compatible = gate-clock;
+clocks = sys_32k_ck;
+bit-shift = 8;
+reg = 0x4a307888 0x4;
+};
Since we already have omap443x.dtsi and omap446x.dtsi, do we need
clock.dtsi containing just a few entries?
instead we could define the delta clocks in the clocks section, and save
on two additional files, no?
Yea, thats also possible. I didn't want to put clock nodes there though,
just for clarity. I think this is for whoever is maintaining the DTS
files to answer.
[...]
diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi
b/arch/arm/boot/dts/omap44xx-clocks.dtsi
new file mode 100644
index 000..ed6bc9b
--- /dev/null
+++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi
[...]
+dpll_abe_m2x2_ck: dpll_abe_m2x2_ck@4a0041f0 {
+#clock-cells = 0;
+compatible = divider-clock;
+clocks = dpll_abe_x2_ck;
+ti,autoidle-shift = 8;
+reg = 0x4a0041f0 0x4;
+bit-mask = 0x1f;
+index-starts-at-one;
+ti,autoidle-low;
+};
+
+abe_24m_fclk: abe_24m_fclk {
+#clock-cells = 0;
+compatible = fixed-factor-clock;
+clocks = dpll_abe_m2x2_ck;
+clock-mult = 1;
+clock-div = 8;
+};
+
+abe_clk: abe_clk@4a004108 {
+#clock-cells = 0;
+compatible = divider-clock;
+clocks = dpll_abe_m2x2_ck;
+reg = 0x4a004108 0x4;
+bit-mask = 0x3;
+index-power-of-two;
+};
+
+aess_fclk: aess_fclk@4a004528 {
is there a naming convention used here? abe_clk, fclk etc?
The clock names are directly converted from existing data, so whatever
currently is there, will be in the DT also.
+#clock-cells = 0;
+compatible = divider-clock;
+clocks = abe_clk;
+bit-shift = 24;
+reg = 0x4a004528 0x4;
+bit-mask = 0x1;
+};
[...]
+
+ocp2scp_usb_phy_phy_48m: ocp2scp_usb_phy_phy_48m@4a0093e0 {
_ck?
[...]
--
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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. Thanks, Richard -- 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: [PATCHv4 09/33] CLK: omap: add omap4 clock init file
On 07/30/2013 10:33 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
clk-44xx.c now contains the clock init functionality for omap4, including
DT clock registration and adding of static clkdev entries.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/clk-44xx.c | 118
+++
1 file changed, 118 insertions(+)
create mode 100644 drivers/clk/omap/clk-44xx.c
diff --git a/drivers/clk/omap/clk-44xx.c b/drivers/clk/omap/clk-44xx.c
new file mode 100644
index 000..cc12134
--- /dev/null
+++ b/drivers/clk/omap/clk-44xx.c
@@ -0,0 +1,118 @@
+/*
+ * OMAP4 Clock data
+ *
+ * Copyright (C) 2009-2012 Texas Instruments, Inc.
+ * Copyright (C) 2009-2010 Nokia Corporation
+ *
+ * Paul Walmsley (p...@pwsan.com)
+ * Rajendra Nayak (rna...@ti.com)
+ * Benoit Cousson (b-cous...@ti.com)
+ * Mike Turquette (mturque...@ti.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * XXX Some of the ES1 clocks have been removed/changed; once support
+ * is added for discriminating clocks by ES level, these should be
added back
+ * in.
+ *
+ * XXX All of the remaining MODULEMODE clock nodes should be removed
+ * once the drivers are updated to use pm_runtime or to use the
appropriate
+ * upstream clock node for rate/parent selection.
+ */
+
+#include linux/kernel.h
+#include linux/list.h
+#include linux/clk-private.h
+#include linux/clkdev.h
+#include linux/io.h
+#include linux/clk/omap.h
+
+/*
+ * OMAP4 ABE DPLL default frequency. In OMAP4460 TRM version V, section
+ * 3.6.3.2.3 CM1_ABE Clock Generator states that the DPLL_ABE_X2_CLK
+ * must be set to 196.608 MHz and hence, the DPLL locked frequency is
+ * half of this value.
+ */
+#define OMAP4_DPLL_ABE_DEFFREQ98304000
+
+/*
+ * OMAP4 USB DPLL default frequency. In OMAP4430 TRM version V, section
+ * 3.6.3.9.5 DPLL_USB Preferred Settings shows that the preferred
+ * locked frequency for the USB DPLL is 960MHz.
+ */
+#define OMAP4_DPLL_USB_DEFFREQ96000
+
+static struct omap_dt_clk omap44xx_clks[] = {
+DT_CLK(smp_twd,NULL,mpu_periphclk),
+DT_CLK(omapdss_dss, ick, dss_fck),
+DT_CLK(usbhs_omap, fs_fck, usb_host_fs_fck),
+DT_CLK(usbhs_omap, hs_fck, usb_host_hs_fck),
+DT_CLK(usbhs_omap, usbtll_ick, usb_tll_hs_ick),
+DT_CLK(usbhs_tll, usbtll_ick, usb_tll_hs_ick),
+DT_CLK(NULL,timer_32k_ck,sys_32k_ck),
+/* TODO: Remove omap_timer.X aliases once DT migration is
complete */
+DT_CLK(omap_timer.1,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.2,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.3,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.4,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.9,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.10,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.11,timer_sys_ck,sys_clkin_ck),
+DT_CLK(omap_timer.5,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(omap_timer.6,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(omap_timer.7,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(omap_timer.8,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(4a318000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(48032000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(48034000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(48036000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(4803e000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(48086000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(48088000.timer,timer_sys_ck,sys_clkin_ck),
+DT_CLK(40138000.timer,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(4013a000.timer,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(4013c000.timer,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(4013e000.timer,timer_sys_ck,syc_clk_div_ck),
+DT_CLK(NULL,cpufreq_ck,dpll_mpu_ck),
please remove cpufreq.
Hmm why?
Because cpufreq is completely broken now and your current work on it? :)
+};
+
+int __init omap4xxx_clk_init(void)
+{
+int rc;
+struct clk *abe_dpll_ref, *abe_dpll, *sys_32k_ck, *usb_dpll;
+
+/* FIXME register clocks from DT first */
+dt_omap_clk_init();
+
+omap_dt_clocks_register(omap44xx_clks, ARRAY_SIZE(omap44xx_clks));
+
+omap2_clk_disable_autoidle_all();
+
+/*
+ * On OMAP4460 the ABE DPLL fails to turn on if in idle low-power
+ * state when turning the ABE clock domain. Workaround this by
+ * locking the ABE DPLL on boot.
+ * Lock the ABE DPLL in any case to avoid issues with audio.
+ */
But this code will be called for 4430 and 4460. if the requirement is
only for 4460, then we are not adhering to the spec?
This is copy pasted from existing cclock44xx_data.c file init function.
+abe_dpll_ref
Re: [PATCHv4 10/33] ARM: OMAP4: remove old clock data and link in new clock init code
On 07/30/2013 10:42 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
diff --git a/arch/arm/mach-omap2/cclock44xx_data.c
b/arch/arm/mach-omap2/cclock44xx_data.c
deleted file mode 100644
index 88e37a4..000
--- a/arch/arm/mach-omap2/cclock44xx_data.c
+++ /dev/null
[...]
-
-int __init omap4xxx_clk_init(void)
-{
arch/arm/mach-omap2/clock44xx.h:int omap4xxx_clk_init(void);
arch/arm/mach-omap2/io.c: omap_clk_init = omap4xxx_clk_init;
code in drivers/clk/omap/clk-44xx.c
Seems goofy to me a little.
entire purpose of having a clk-44xx.c is:
a) doing a clk alias for device nodes
I am not quite sure we have mechanisms for doing this (yet).
b) set_parent, rate
I think this is maybe an idea for future dev and Mike to consider if he
wants generic clock nodes to have such properties.
both of these seem to be an old style carry forward and should instead
be fixes with generic properties IMHO voiding the need for SoC specific
inits.
instead all we should be doing is call of_clk_init(NULL); at appropriate
init sequence.
--
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: [PATCHv4 15/33] CLK: OMAP: DPLL: add support for DT property ti,dpll-no-gate
On 07/30/2013 10:18 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
AM335x has DPLL clocks that should never be attempted to be gated. Adding
ti,dpll-no-gate property for them handles this situation.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/dpll.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
index 66e82be..1d24feada 100644
--- a/drivers/clk/omap/dpll.c
+++ b/drivers/clk/omap/dpll.c
@@ -54,6 +54,13 @@ static const struct clk_ops dpll_x2_ck_ops = {
.recalc_rate= omap3_clkoutx2_recalc,
};
+static const struct clk_ops dpll_no_gate_ck_ops = {
+.recalc_rate= omap3_dpll_recalc,
+.get_parent= omap2_init_dpll_parent,
+.round_rate= omap2_dpll_round_rate,
+.set_rate= omap3_noncore_dpll_set_rate,
+};
+
struct clk *omap_clk_register_dpll(struct device *dev, const char
*name,
const char **parent_names, int num_parents, unsigned long
flags,
struct dpll_data *dpll_data, const char *clkdm_name,
@@ -288,6 +295,9 @@ __init void of_omap4_dpll_setup(struct device_node
*node)
return;
}
+if (of_property_read_bool(node, ti,dpll-no-gate))
+ops = dpll_no_gate_ck_ops;
+
of_omap_dpll_setup(node, ops);
}
EXPORT_SYMBOL_GPL(of_omap4_dpll_setup);
squash this to dpll patch?
Can do it. Was kept separate just to avoid confusion with previous rev
of the code.
-Tero
--
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: [PATCHv4 16/33] CLK: OMAP: DPLL: do not of_iomap NULL autoidle register
On 07/30/2013 10:49 PM, Nishanth Menon wrote: On 07/23/2013 02:20 AM, Tero Kristo wrote: AM33xx series SoCs do not have autoidle support, and for these the autoidle register is marked as NULL. Check against a NULL pointer and do not attempt to of_iomap in this case, as this just creates a bogus pointer and causes a kernel crash during boot. Signed-off-by: Tero Kristo t-kri...@ti.com --- drivers/clk/omap/dpll.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c index 1d24feada..d8a958a 100644 --- a/drivers/clk/omap/dpll.c +++ b/drivers/clk/omap/dpll.c @@ -162,6 +162,7 @@ static void __init of_omap_dpll_setup(struct device_node *node, u32 max_multiplier = 2047; u32 max_divider = 128; u32 min_divider = 1; +u32 val; int i; dd = kzalloc(sizeof(struct dpll_data), GFP_KERNEL); @@ -210,7 +211,14 @@ static void __init of_omap_dpll_setup(struct device_node *node, dd-control_reg = of_iomap(node, 0); dd-idlest_reg = of_iomap(node, 1); -dd-autoidle_reg = of_iomap(node, 2); +/* + * AM33xx DPLLs have no autoidle support, and the autoidle reg + * for these is NULL. Do not attempt to of_iomap in this case, + * as this just creates a bogus pointer and crashes the kernel. + */ +of_property_read_u32_index(node, reg, 2 * 2, val); +if (val) +dd-autoidle_reg = of_iomap(node, 2); should we not do that for all the parameters? Maybe do the check for all. OR: move this as index 3 which is optional and is not defined for am33xx? dd-mult_div1_reg = of_iomap(node, 3); dd-idlest_mask = idlest_mask; we could probably squash this in original dpll.c as a result? Yea, can do that also. -- 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: [PATCHv4 19/33] CLK: omap: add am33xx clock init file
On 07/30/2013 11:00 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
clk-33xx.c now contains the clock init functionality for am33xx,
including
DT clock registration and adding of static clkdev entries.
This patch also moves the omap2_clk_enable_init_clocks declaration to
the driver include, as this is needed by the am33xx clock init code.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
arch/arm/mach-omap2/clock.h |1 -
drivers/clk/omap/clk-33xx.c | 85
+++
include/linux/clk/omap.h|1 +
3 files changed, 86 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/omap/clk-33xx.c
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index d1a3125..6273f14 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -267,7 +267,6 @@ void omap2_clk_dflt_find_idlest(struct clk_hw_omap
*clk,
void __iomem **idlest_reg,
u8 *idlest_bit, u8 *idlest_val);
int omap2_clk_enable_autoidle_all(void);
-void omap2_clk_enable_init_clocks(const char **clk_names, u8
num_clocks);
int omap2_clk_switch_mpurate_at_boot(const char *mpurate_ck_name);
void omap2_clk_print_new_rates(const char *hfclkin_ck_name,
const char *core_ck_name,
diff --git a/drivers/clk/omap/clk-33xx.c b/drivers/clk/omap/clk-33xx.c
new file mode 100644
index 000..3ada30e
--- /dev/null
+++ b/drivers/clk/omap/clk-33xx.c
[...]
+static const char *enable_init_clks[] = {
+dpll_ddr_m2_ck,
+dpll_mpu_m2_ck,
+l3_gclk,
+l4hs_gclk,
+l4fw_gclk,
+l4ls_gclk,
+/* Required for external peripherals like, Audio codecs */
+clkout2_ck,
+};
should be a sort of dt property?
Future dev maybe?
I try to avoid adding too many new props with this set
-Tero
--
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: [PATCHv4 21/33] CLK: OMAP: DPLL: add omap3 dpll support
On 07/30/2013 11:08 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
OMAP3 has slightly different DPLLs from those compared to OMAP4. Modified
code for the same.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/dpll.c | 96
+--
1 file changed, 85 insertions(+), 11 deletions(-)
:) wont repeat the binding crib again..
diff --git a/drivers/clk/omap/dpll.c b/drivers/clk/omap/dpll.c
index d8a958a..ecb1fbd 100644
--- a/drivers/clk/omap/dpll.c
+++ b/drivers/clk/omap/dpll.c
@@ -26,6 +26,11 @@
#include linux/of_address.h
#include linux/clk/omap.h
+enum {
+SUBTYPE_OMAP3_DPLL,
+SUBTYPE_OMAP4_DPLL,
+};
+
static const struct clk_ops dpll_m4xen_ck_ops = {
.enable= omap3_noncore_dpll_enable,
.disable= omap3_noncore_dpll_disable,
@@ -40,6 +45,13 @@ static const struct clk_ops dpll_core_ck_ops = {
.get_parent= omap2_init_dpll_parent,
};
+static const struct clk_ops omap3_dpll_core_ck_ops = {
+.init= omap2_init_clk_clkdm,
+.get_parent= omap2_init_dpll_parent,
+.recalc_rate= omap3_dpll_recalc,
+.round_rate= omap2_dpll_round_rate,
+};
+
static const struct clk_ops dpll_ck_ops = {
.enable= omap3_noncore_dpll_enable,
.disable= omap3_noncore_dpll_disable,
@@ -50,6 +62,26 @@ static const struct clk_ops dpll_ck_ops = {
.init= omap2_init_clk_clkdm,
};
+static const struct clk_ops omap3_dpll_ck_ops = {
+.init= omap2_init_clk_clkdm,
+.enable= omap3_noncore_dpll_enable,
+.disable= omap3_noncore_dpll_disable,
+.get_parent= omap2_init_dpll_parent,
+.recalc_rate= omap3_dpll_recalc,
+.set_rate= omap3_noncore_dpll_set_rate,
+.round_rate= omap2_dpll_round_rate,
+};
+
+static const struct clk_ops omap3_dpll_per_ck_ops = {
+.init= omap2_init_clk_clkdm,
+.enable= omap3_noncore_dpll_enable,
+.disable= omap3_noncore_dpll_disable,
+.get_parent= omap2_init_dpll_parent,
+.recalc_rate= omap3_dpll_recalc,
+.set_rate= omap3_dpll4_set_rate,
+.round_rate= omap2_dpll_round_rate,
+};
+
static const struct clk_ops dpll_x2_ck_ops = {
.recalc_rate= omap3_clkoutx2_recalc,
};
@@ -144,7 +176,9 @@ struct clk *omap_clk_register_dpll_x2(struct
device *dev, const char *name,
* of_omap_dpll_setup() - Setup function for OMAP DPLL clocks
*/
static void __init of_omap_dpll_setup(struct device_node *node,
-const struct clk_ops *ops)
+const struct clk_ops *ops, u32 freqsel,
+u32 modes, u8 mul_div_shift,
+int subtype)
{
struct clk *clk;
const char *clk_name = node-name;
@@ -157,8 +191,8 @@ static void __init of_omap_dpll_setup(struct
device_node *node,
u32 idlest_mask = 0x1;
u32 enable_mask = 0x7;
u32 autoidle_mask = 0x7;
-u32 mult_mask = 0x7ff 8;
-u32 div1_mask = 0x7f;
+u32 mult_mask = 0x7ff (8 + mul_div_shift);
+u32 div1_mask = 0x7f mul_div_shift;
u32 max_multiplier = 2047;
u32 max_divider = 128;
u32 min_divider = 1;
@@ -193,7 +227,7 @@ static void __init of_omap_dpll_setup(struct
device_node *node,
clkspec.np = of_parse_phandle(node, ti,clk-ref, 0);
dd-clk_ref = of_clk_get_from_provider(clkspec);
-if (!dd-clk_ref) {
+if (IS_ERR(dd-clk_ref)) {
belongs to original patch.
Agree.
pr_err(%s: ti,clk-ref for %s not found\n, __func__,
clk_name);
goto cleanup;
@@ -201,7 +235,7 @@ static void __init of_omap_dpll_setup(struct
device_node *node,
clkspec.np = of_parse_phandle(node, ti,clk-bypass, 0);
dd-clk_bypass = of_clk_get_from_provider(clkspec);
-if (!dd-clk_bypass) {
+if (IS_ERR(dd-clk_bypass)) {
same
Ditto.
pr_err(%s: ti,clk-bypass for %s not found\n, __func__,
clk_name);
goto cleanup;
@@ -225,14 +259,31 @@ static void __init of_omap_dpll_setup(struct
device_node *node,
dd-enable_mask = enable_mask;
dd-autoidle_mask = autoidle_mask;
-dd-modes = 0xa0;
+if (!of_property_read_u32(node, ti,recal-en-bit, val))
+dd-recal_en_bit = val;
+
+if (!of_property_read_u32(node, ti,recal-st-bit, val))
+dd-recal_st_bit = val;
+
+if (!of_property_read_u32(node, ti,auto-recal-bit, val))
+dd-auto_recal_bit = val;
now I understand what it means.
I am not quite sure you do, as I don't quite get your comment here. :)
You referring to that dd-modes part?
+
+of_property_read_u32(node, ti,modes, modes);
i see we pass in modes, and read ti,modes to modes. it is a bit sketchy
without bindings documentation.
ti,modes can be used to override the default modes.
+
+dd-modes = modes;
Should have belonged to original patch.
If I squash this then we are fine.
+
+dd-freqsel_mask =
Re: [PATCHv4 22/33] CLK: OMAP: update gate clock setup for OMAP3
On 07/30/2013 11:13 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
OMAP3 gate clocks are handled through the clk driver now. Basic gate
clock can't be used as the OMAP3 gate clocks have some special features,
namely the idle status linkage which is on separate register.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/gate.c | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/omap/gate.c b/drivers/clk/omap/gate.c
index 7186bb2..b560ff4 100644
--- a/drivers/clk/omap/gate.c
+++ b/drivers/clk/omap/gate.c
@@ -28,12 +28,19 @@
#ifdef CONFIG_OF
-static const struct clk_ops omap_gate_clk_ops = {
+static const struct clk_ops omap_gate_clkdm_clk_ops = {
.init= omap2_init_clk_clkdm,
.enable= omap2_clkops_enable_clkdm,
.disable= omap2_clkops_disable_clkdm,
};
+static const struct clk_ops omap_gate_clk_ops = {
+.init= omap2_init_clk_clkdm,
+.enable= omap2_dflt_clk_enable,
+.disable= omap2_dflt_clk_disable,
+.is_enabled= omap2_dflt_clk_is_enabled,
+};
+
void __init of_omap_gate_clk_setup(struct device_node *node)
{
struct clk *clk;
@@ -43,6 +50,7 @@ void __init of_omap_gate_clk_setup(struct
device_node *node)
int num_parents;
const char **parent_names;
int i;
+u32 val;
clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);
if (!clk_hw) {
@@ -56,7 +64,22 @@ void __init of_omap_gate_clk_setup(struct
device_node *node)
of_property_read_string(node, ti,clkdm-name,
clk_hw-clkdm_name);
init.name = clk_name;
-init.ops = omap_gate_clk_ops;
+init.flags = 0;
+
+if (of_property_read_u32_index(node, reg, 0, val)) {
+/* No register, clkdm control only */
+init.ops = omap_gate_clkdm_clk_ops;
+} else {
+init.ops = omap_gate_clk_ops;
+clk_hw-enable_reg = of_iomap(node, 0);
+of_property_read_u32(node, ti,enable-bit, val);
+clk_hw-enable_bit = val;
+
+if (of_property_read_bool(node, ti,dss-clk))
+clk_hw-ops = clkhwops_omap3430es2_dss_usbhost_wait;
umm, it was going relatively ok so far, till i hit this :( it is
probably a quirk... but still..
Some of the clocks need special hwops for them to work properly it
seems... It looks nasty yea but the best I could think of.
+else
+clk_hw-ops = clkhwops_wait;
+}
num_parents = of_clk_get_parent_count(node);
if (num_parents 1) {
but still no usage of ti,omap-gate-clock makes me question the need
for this file.
Yea, no ti,omap-gate-clock, but there is ti,gate-clock. Just look harder.
--
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: [PATCHv4 23/33] CLK: OMAP: add interface clock support for OMAP3
On 07/30/2013 11:23 PM, Nishanth Menon wrote:
On 07/23/2013 02:20 AM, Tero Kristo wrote:
OMAP3 has interface clocks in addition to functional clocks, which
is it just OMAP3?
Yea, only omap3 is using this code. Basically because there is control
for the module specific interface clocks which is absent from omap4+.
Personally I think modelling the interface clocks in the first place in
kernel side was a bad idea, and should have just enabled all of them and
enable autoidles for them at the same point.
require special handling for the autoidle and idle status register
offsets mainly.
Signed-off-by: Tero Kristo t-kri...@ti.com
---
drivers/clk/omap/Makefile|2 +-
drivers/clk/omap/clk.c |3 ++
drivers/clk/omap/interface.c | 110
++
should this be isolated off for omap3?
You mean within makefile or?
3 files changed, 114 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/omap/interface.c
diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
index c4e8825..faaeb62 100644
--- a/drivers/clk/omap/Makefile
+++ b/drivers/clk/omap/Makefile
@@ -1,3 +1,3 @@
obj-y+= clk.o dpll.o autoidle.o gate.o \
clk-44xx.o clk-54xx.o clk-7xx.o \
- clk-33xx.o
+ clk-33xx.o interface.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
index 8c89714..5cbefde 100644
--- a/drivers/clk/omap/clk.c
+++ b/drivers/clk/omap/clk.c
@@ -30,6 +30,9 @@ static const struct of_device_id clk_match[] = {
{.compatible = gate-clock, .data = of_gate_clk_setup, },
{.compatible = ti,omap4-dpll-clock, .data =
of_omap4_dpll_setup, },
{.compatible = ti,gate-clock, .data = of_omap_gate_clk_setup, },
+{.compatible = ti,interface-clock,
+.data = of_omap_interface_clk_setup, },
+{.compatible = ti,omap3-dpll-clock, .data =
of_omap3_dpll_setup, },
I dont see how this line has anything to do with the patch.
Yea, seems it should have been part of the omap3 dpll introduction one.
{},
};
diff --git a/drivers/clk/omap/interface.c b/drivers/clk/omap/interface.c
new file mode 100644
index 000..f1f1a1a
--- /dev/null
+++ b/drivers/clk/omap/interface.c
@@ -0,0 +1,110 @@
+/*
+ * OMAP interface clock support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk-provider.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/io.h
+#include linux/err.h
+#include linux/string.h
+#include linux/log2.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/clk/omap.h
+
+#ifdef CONFIG_OF
+
+static const struct clk_ops omap_interface_clk_ops = {
+.init= omap2_init_clk_clkdm,
+.enable= omap2_dflt_clk_enable,
+.disable= omap2_dflt_clk_disable,
+.is_enabled= omap2_dflt_clk_is_enabled,
+};
+
+void __init of_omap_interface_clk_setup(struct device_node *node)
+{
+struct clk *clk;
+struct clk_init_data init;
+struct clk_hw_omap *clk_hw;
+const char *clk_name = node-name;
+int num_parents;
+const char **parent_names;
+int i;
+u32 val;
+
+clk_hw = kzalloc(sizeof(struct clk_hw_omap), GFP_KERNEL);
+if (!clk_hw) {
+pr_err(%s: could not allocate clk_hw_omap\n, __func__);
+return;
+}
+
+clk_hw-hw.init = init;
+clk_hw-ops = clkhwops_iclk_wait;
+clk_hw-enable_reg = of_iomap(node, 0);
+
+if (!of_property_read_u32(node, ti,enable-bit, val))
+clk_hw-enable_bit = val;
+
+if (of_property_read_bool(node, ti,iclk-no-wait))
+clk_hw-ops = clkhwops_iclk;
+
+if (of_property_read_bool(node, ti,iclk-hsotgusb))
+clk_hw-ops = clkhwops_omap3430es2_iclk_hsotgusb_wait;
+
+if (of_property_read_bool(node, ti,iclk-dss))
+clk_hw-ops = clkhwops_omap3430es2_iclk_dss_usbhost_wait;
+
+if (of_property_read_bool(node, ti,iclk-ssi))
+clk_hw-ops = clkhwops_omap3430es2_iclk_ssi_wait;
+
+of_property_read_string(node, clock-output-names, clk_name);
+of_property_read_string(node, ti,clkdm-name, clk_hw-clkdm_name);
+
+init.name = clk_name;
+init.ops = omap_interface_clk_ops;
+init.flags = 0;
+
+num_parents = of_clk_get_parent_count(node);
+if (num_parents 1) {
+pr_err(%s: omap interface_clk %s must have parent(s)\n,
+__func__, node-name);
+goto cleanup;
+}
+
+parent_names = kzalloc(sizeof(char *) * num_parents,
Re: [PATCHv4 29/33] CLK: omap: add omap3 clock init file
On 07/31/2013 09:35 AM, Tony Lindgren wrote: * Nishanth Menon n...@ti.com [130730 13:26]: On 07/23/2013 02:20 AM, Tero Kristo wrote: clk-3xxx.c now contains the clock init functionality for omap3, including DT clock registration and adding of static clkdev entries. This patch also splits the OMAP3 clock registration code under mach-omap2 to use OMAP3 subtype specific clk init functions. Signed-off-by: Tero Kristo t-kri...@ti.com --- arch/arm/mach-omap2/Makefile |2 +- arch/arm/mach-omap2/cclock3xxx_data.c | 3641 - Tony and a lot of people is not going to like removing support for non-dt boot for OMAP3 before it's time is due ;) I think the only showstopper for that is that we need the pending pinctrl changes merged first to keep off-idle working. So the omap3 legacy code removal probably needs to wait until v3.13 merge window. For omap4 and am33xx we can do it now. Regards, Tony I'll modify the series in such way that OMAP3 retains the legacy clock data within the kernel for now, and will use either DT or kernel data based on init type. Kernel data can then be removed when time is ripe for it. -Tero -- 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] of: provide of_platform_unpopulate()
* Grant Likely | 2013-07-24 15:19:58 [+0100]: Was there more breakage than imx6 and amba devices? Your first version had a fallback case for powerpc. Couldn't we do just allow that for more than just powerpc? I'd much rather see some work-around within the core DT code with a warning to prevent more proliferation than putting this into drivers. It's tricky stuff. I've not figured out a solution I'm happy with. Trying to figure out when to apply a work around is hard because the resource reservation makes assumptions about the memory range layout that doesn't match the assumptions made by device tree code. I can't really follow. Do you have a simple at hand? One /possible/ option is to not add the resources to the devices at all when the device is registered and instead resolve them right at bind time. Jean Christophe proposed doing this already to solve a different problem; obtaining resources that require other drivers to be probed first. If the resources are resolved at .probe() time, then the resource registration problem should also go away. The downside to that approach is that it makes each deferred probe more expensive; potentially a *lot* more expensive depending on how much work the xlate functions have to do. It would be worth prototyping though to see how well it works. So you say defer the io ressources until the device-tree device is actually probed. I don't really understand why that defer part should solve the problem but I would try and see how it goes. Jean-Christophe proposed that only, that means no patches yet, right? g. Sebastian -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 04:49:59PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. -- balbi signature.asc Description: Digital signature
Re: [PATCH] of: provide of_platform_unpopulate()
On 07/29/2013 04:33 AM, Benjamin Herrenschmidt wrote: On Mon, 2013-07-22 at 00:44 +0100, Grant Likely wrote: BTW, it looks like Grant has attempted this already: Yup, things broke badly. Unfortunately the of_platform_device and platform_device history doesn't treat resources in the same way. I would like to merge the code, but I haven't been able to figure out a clean way to do it. Looks like we do need the unpopulate function. What is the exact problem Grant ? Care to give me an example ? See this thread: http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg63678.html Rob -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 06:28:27PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 04:49:59PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. And if the driver reads junk from some random address due to bootloader/DT/multikernel madness, it will happily peek and poke around instead of rejecting the wrong version number. Thanks, Richard -- 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] ARM: allow DEBUG_UNCOMPRESS for omap2plus
On 07/31/2013 12:46 AM, Tony Lindgren wrote: * Stephen Warren swar...@wwwdotorg.org [130730 16:08]: On 07/30/2013 04:52 PM, Russell King - ARM Linux wrote: On Tue, Jul 30, 2013 at 04:49:18PM -0600, Stephen Warren wrote: From: Stephen Warren swar...@nvidia.com DEBUG_UNCOMPRESS was previously disallowed for omap2plus due to omap2plus.S's use of .data, which is not allowed in the decompressor. Solve this by placing that data into .text when building the file into the decompressor. This relies on .text actually being writable in the decompressor, which it is in practice. Unless you decide to use ZBOOT and flash the zImage. I knew there had to be a catch:-) I have no idea if ZBOOT is a use-case that's relevant to OMAP? On Tegra at least (the same issue applies to the other patch I just sent), that use-case is almost impossible; even if the boot ROM directly booted a kernel, the boot ROM is hard-coded to copy whatever it's booting to SDRAM first, although I suppose if that was a boot-loader it could just jump back to a ROM location. That said, NOR flash is extremely rare on Tegra. So, I don't know if we care about this issue. Is it reasonable to just say If you use ZBOOT, don't enable DEBUG_UNCOMPRESS? Perhaps these patches should not completely remove the !DEBUG_TEGRA_UART from config DEBUG_UNCOMPRESS, but instead say: default y if DEBUG_LL (!DEBUG_TEGRA_UART || !ZBOOT)? I think we're best off removing the remaining uncompress code configured port detection features as the port properties are now defined in kconfig anyways. That simplifies the code quite a bit. If you want to do that with OMAP, I'm happy to drop this patch. For Tegra, automatic determination of the DEBUG_LL UART is rather useful, so I'm not going to give that up:-) -- 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: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
On Tue, Jul 30, 2013 at 10:47 PM, Sourav Poddar sourav.pod...@ti.com wrote:
Test details:
-
Tested this on dra7 board.
Test1: Ran mtd_stesstest for 4 iterations.
- All iterations went through without failure.
Test2: Use mtd utilities:
- flash_erase to erase the flash device
- nanddump to read data back.
- nandwrite to write to the data flash.
diff between the write and read data shows zero.
You've obviously never tested word lengths other than 8, because...
+static inline void ti_qspi_read_data(struct ti_qspi *qspi,
+ unsigned long reg, int wlen, u8 **rxbuf)
+{
+ switch (wlen) {
+ case 8:
+ **rxbuf = readb(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %02x\n, *(*rxbuf));
+ *rxbuf += 1;
+ break;
+ case 16:
+ **rxbuf = readw(qspi-base + reg);
*rxbuf is a u8*. This means when you assign to **rxbuf the type of
the lvalue is u8. 8 bits. It does not matter what type the rvalue
is, u8, u16, or u32, the result will always be truncated to 8 bits.
IMHO, I toss the design of ti_qspi_read/write_data(). They look like
a function to read a generic register, yet it only makes sense with
QSPI_SPI_DATA_REG. Passing the pointer by address so it can be
incremented is ugly. And doesn't work at all, since the type of the
pointer isn't going to be the same for different word lengths. The
case statement inside the inner loop is inefficient. Each case is
largely duplicated code.
--
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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
Hi, On Wed, Jul 31, 2013 at 06:38:46PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 06:28:27PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 04:49:59PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. And if the driver reads junk from some random address due to bootloader/DT/multikernel madness, it will happily peek and poke around instead of rejecting the wrong version number. that'd be a bug in the DT anyway, why should the driver have to cope with broken data ? -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 09:45:25PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 06:38:46PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 06:28:27PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 04:49:59PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. And if the driver reads junk from some random address due to bootloader/DT/multikernel madness, it will happily peek and poke around instead of rejecting the wrong version number. that'd be a bug in the DT anyway, why should the driver have to cope with broken data ? Um, it is called error checking? Besides, by not checking the version number, you are pre-programming a disaster that will occur when an older kernel is booted on the first new IP version with important changes. Can you really be sure that all users will have the new, patched kernel? Thanks, Richard -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
Hi, On Wed, Jul 31, 2013 at 09:22:29PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 09:45:25PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 06:38:46PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 06:28:27PM +0300, Felipe Balbi wrote: On Wed, Jul 31, 2013 at 04:49:59PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 05:42:26PM +0530, Mugunthan V N wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. And if the driver reads junk from some random address due to bootloader/DT/multikernel madness, it will happily peek and poke around instead of rejecting the wrong version number. that'd be a bug in the DT anyway, why should the driver have to cope with broken data ? Um, it is called error checking? right, now go check on the archives what Linus (and many others, for that matter) have said about version checking. If it's not the version you expect, you assume the latest. Imagine the situation where new IP version has new features, but all the others still work and we don't use that new feature. We'd have to patch the kernel just to get the driver to probe() even though the entire driver is still 'compliant' with the new IP. Besides, by not checking the version number, you are pre-programming a disaster that will occur when an older kernel is booted on the first new IP version with important changes. Can you really be sure that all users will have the new, patched kernel? why will there be a disaster ? -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
Hi, On Wed, Jul 31, 2013 at 10:43:32PM +0300, Felipe Balbi wrote: The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. This doesn't make any sense to me. Why not just add the new version number? None of the hunks in your patch are on performance sensitive paths, so I really can't see any point in removing the error checking. well, if a new revision of the IP comes, the driver at least has some chance to work without having to be modified. If it turns out that there are really different features, then we patch a new version, otherwise we should just assume highest known version and try it out. And if the driver reads junk from some random address due to bootloader/DT/multikernel madness, it will happily peek and poke around instead of rejecting the wrong version number. that'd be a bug in the DT anyway, why should the driver have to cope with broken data ? Um, it is called error checking? one more thing, why do you consider a new revision to be an error ? -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 10:45:23PM +0300, Felipe Balbi wrote: one more thing, why do you consider a new revision to be an error ? Okay, so why don't you go and remove the version checking code altogether? Thanks, Richard -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
Hi, On Wed, Jul 31, 2013 at 10:04:28PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 10:45:23PM +0300, Felipe Balbi wrote: one more thing, why do you consider a new revision to be an error ? Okay, so why don't you go and remove the version checking code altogether? if you need to treak certain aspects of the IP differently, you need the revision check, don't be so childish. what I'm saying is that we can give new IP revision a chance to work if they have no programming model differences (except for, perhaps, new features and different erratas). On dwc3 (drivers/usb/dwc3) we support every single revision of the IP. We only have revision checks to enable (or not) known silicon erratas. -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 11:07:56PM +0300, Felipe Balbi wrote: what I'm saying is that we can give new IP revision a chance to work if they have no programming model differences (except for, perhaps, new features and different erratas). But it also has a chance to fail when there are differences. Comparing CPSW V1 with V2, it appears that TI likes to move the registers around between versions. To me, this is reason enough to make the driver defensive. Thanks, Richard -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
Hi, On Wed, Jul 31, 2013 at 10:20:07PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 11:07:56PM +0300, Felipe Balbi wrote: what I'm saying is that we can give new IP revision a chance to work if they have no programming model differences (except for, perhaps, new features and different erratas). But it also has a chance to fail when there are differences. Comparing CPSW V1 with V2, it appears that TI likes to move the registers around between versions. To me, this is reason enough to make the driver defensive. oh well, we can go on and on with this. Unfortunately we (SW team) don't have control over the HW folks. We strongly suggest that they don't break SW compatibility, and that's starting to become true. You can very well expect next version of CPSW to be SW compatible. If it isn't, then TI will send patches to add a new revision check and treat it well. We are the first ones to have access to new versions of all our IPs anyway. And, IMHO, even if HW engineers decides to move registers around in CPSW v3, that still doesn't chage the fact that defaulting to highest known revision is a good practice. Bailing out just because the revision check isn't what you expect it to be is a very poor practice and leads to periodic patches updating 'switch' statements all over the place. -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 11:26:17PM +0300, Felipe Balbi wrote: oh well, we can go on and on with this. Unfortunately we (SW team) don't have control over the HW folks. We strongly suggest that they don't break SW compatibility, and that's starting to become true. You can very well expect next version of CPSW to be SW compatible. If it isn't, then TI will send patches to add a new revision check and treat it well. We are the first ones to have access to new versions of all our IPs anyway. Okay, so starting with V3 the registers probably won't be moving around any more. But at the very least your patch should include macros for the known V3 along with the default case. Thanks, Richard -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 10:34:06PM +0200, Richard Cochran wrote: On Wed, Jul 31, 2013 at 11:26:17PM +0300, Felipe Balbi wrote: oh well, we can go on and on with this. Unfortunately we (SW team) don't have control over the HW folks. We strongly suggest that they don't break SW compatibility, and that's starting to become true. You can very well expect next version of CPSW to be SW compatible. If it isn't, then TI will send patches to add a new revision check and treat it well. We are the first ones to have access to new versions of all our IPs anyway. Okay, so starting with V3 the registers probably won't be moving around any more. But at the very least your patch should include macros for the known V3 along with the default case. that's the point, there is no known V3. Once it has, surely we will add such macros, but until then, we let the driver assume the highest known revision if it finds a register with an unknown revision. -- balbi signature.asc Description: Digital signature
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
From: Mugunthan V N mugunthan...@ti.com Date: Wed, 31 Jul 2013 17:42:26 +0530 The new IP version has a minor changes and the offsets are same as the previous version, so instead of adding CPSW version number in the driver, make the driver to fall through to the latest versions so that the new version of CPSW which has the same register offsets will work directly without patching the driver. Signed-off-by: Mugunthan V N mugunthan...@ti.com Reviewed-by: Felipe Balbi ba...@ti.com Like others, I really think you should check the version explicitly. Please respin this patch so that it supports new IP versions in that way. Thanks. -- 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 0/4] crypto: omap-sham: Add support for SHA384/SHA512 for OMAP5/AM43xx Soc's
On Fri, Jul 26, 2013 at 12:29:13PM +0530, Lokesh Vutla wrote: This patch series adds support for SHA348 and SHA512 in addition to MD5, SHA1, SHA224 SHA256 that the omap sha module supports. Also adding the pdata for OMAP5 and AM43xx Soc's. And using devm_* calls to make cleanup paths simpler. All applied. Thanks! -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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 7/9] ARM: edma: Don't clear EMR of channel in edma_stop
On 07/31/2013 04:35 AM, Sekhar Nori wrote: On Wednesday 31 July 2013 10:35 AM, Joel Fernandes wrote: On 07/30/2013 03:29 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: We certainly don't want error conditions to be cleared anywhere 'anywhere' is a really loaded term. as this will make us 'forget' about missed events. We depend on knowing which events were missed in order to be able to reissue them. This fixes a race condition where the EMR was being cleared by the transfer completion interrupt handler. Basically, what was happening was: Missed event | | V SG1-SG2-SG3-Null \ \__TC Interrupt (Almost same time as ARM is executing TC interrupt handler, an event got missed and also forgotten by clearing the EMR). Sorry, but I dont see how edma_stop() is coming into picture in the race you describe? In edma_callback function, for the case of DMA_COMPLETE (Transfer completion interrupt), edma_stop() is called when all sets have been processed. This had the effect of clearing the EMR. Ah, thanks. I was missing the fact that the race comes into picture only when using the DMA engine driver. I guess that should be mentioned somewhere since it is not immediately obvious. The patch looks good to me. So if you respin just this one with some updated explanation based on what you wrote below, I will take it. Sure I'll do that. Also the trigger_channel patch, will you be taking that one too? I can send these 2 in a series as they touch arch/arm/common/edma.c Thanks, -Joel Thanks, Sekhar -- 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 -- 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 4/9] dma: edma: Find missed events and issue them
On 07/31/2013 04:18 AM, Sekhar Nori wrote: On Wednesday 31 July 2013 10:19 AM, Joel Fernandes wrote: Hi Sekhar, On 07/30/2013 02:05 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: In an effort to move to using Scatter gather lists of any size with EDMA as discussed at [1] instead of placing limitations on the driver, we work through the limitations of the EDMAC hardware to find missed events and issue them. The sequence of events that require this are: For the scenario where MAX slots for an EDMA channel is 3: SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - Null The above SG list will have to be DMA'd in 2 sets: (1) SG1 - SG2 - SG3 - Null (2) SG4 - SG5 - SG6 - Null After (1) is succesfully transferred, the events from the MMC controller donot stop coming and are missed by the time we have setup the transfer for (2). So here, we catch the events missed as an error condition and issue them manually. Are you sure there wont be any effect of these missed events on the peripheral side. For example, wont McASP get into an underrun condition when it encounters a null PaRAM set? Even UART has to transmit to a But it will not encounter null PaRAM set because McASP uses contiguous buffers for transfer which are not scattered across physical memory. This can be accomplished with an SG of size 1. For such SGs, this patch series leaves it linked Dummy and does not link to Null set. Null set is only used for SG lists that are MAX_NR_SG in size such as those created for example by MMC and Crypto. particular baud so I guess it cannot wait like the way MMC/SD can. Existing driver have to wait anyway if they hit MAX SG limit today. If they don't want to wait, they would have allocated a contiguous block of memory and DMA that in one stretch so they don't lose any events, and in such cases we are not linking to Null. As long as DMA driver can advertize its MAX SG limit, peripherals can always work around that by limiting the number of sync events they generate so as to not having any of the events getting missed. With this series, I am worried that EDMA drivers is advertizing that it can handle any length SG list while not taking care of missing any events while doing so. This will break the assumptions that driver writers make. This is already being done by some other DMA engine drivers ;). We can advertise more than we can handle at a time, that's the basis of this whole idea. I understand what you're saying but events are not something that have be serviced immediately, they can be queued etc and the actually transfer from the DMA controller can be delayed. As long as we don't miss the event we are fine which my series takes care off. So far I have tested this series on following modules in various configurations and have seen no issues: - Crypto AES - MMC/SD - SPI (128x160 display) Also, wont this lead to under-utilization of the peripheral bandwith? Meaning, MMC/SD is ready with data but cannot transfer because the DMA is waiting to be set-up. But it is waiting anyway even today. Currently based on MAX segs, MMC driver/subsystem will make SG list of size max_segs. Between these sessions of creating such smaller SG-lists, if for some reason the MMC controller is sending events, these will be lost anyway. But if MMC/SD driver knows how many events it should generate if it knows the MAX SG limit. So there should not be any missed events in current code. And I am not claiming that your solution is making matters worse. But its not making it much better as well. This is not true for crypto, the events are not deasserted and crypto continues to send events. This is what led to the don't trigger in Null patch where I'm setting the missed flag to avoid recursion. This can be used only for buffers that are contiguous in memory, not those that are scattered across memory. I was hinting at using the linking facility of EDMA to achieve this. Each PaRAM set has full 32-bit source and destination pointers so I see no reason why non-contiguous case cannot be handled. Lets say you need to transfer SG[0..6] on channel C. Now, PaRAM sets are typically 4 times the number of channels. In this case we use one DMA PaRAM set and two Link PaRAM sets per channel. P0 is the DMA PaRAM set and P1 and P2 are the Link sets. Initial setup: SG0 - SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - NULL ^ ^ ^ | | | P0 - P1 - P2 - NULL P[0..2].TCINTEN = 1, so get an interrupt after each SG element completion. On each completion interrupt, hardware automatically copies the linked PaRAM set into the DMA PaRAM set so after SG0 is transferred out, the state of hardware is: SG1 - SG2 - SG3 - SG3 - SG6 - NULL ^ ^ | | P0,1P2 - NULL | ^ | | - SG1 transfer has already started by the time the TC interrupt is handled. As you can see P1 is now redundant and ready to be recycled. So
[PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context.
We found a problem when we removed a working sd card that the irqaction
of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work.
In func omap_hsmmc_reset_controller_fsm, it should watch a 0-1
transition.It used loops_per_jiffy as the timer.
The code is:
while ((!(OMAP_HSMMC_READ(host-base, SYSCTL) bit))
(i++ limit))
cpu_relax();
But the loops_per_jiffy is:
while(i++ limit)
cpu_relax();
It add some codes so the time became long.
Becasue those codes in ISR context, it can't use timer_before/after.
I divived the time into 1ms and used udelay(1) to instead.
It will cause do additional udelay(1).But from my test,it looks good.
Reported-by: Yuzheng Ma mayuzh...@kedacom.com
Tested-by: Yuzheng Ma mayuzh...@kedacom.com
Signed-off-by: Jianpeng Ma majianp...@gmail.com
---
drivers/mmc/host/omap_hsmmc.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 1865321..96daca1 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct
omap_hsmmc_host *host,
unsigned long limit = (loops_per_jiffy *
msecs_to_jiffies(MMC_TIMEOUT_MS));
+ /*Divided time into us for unit 1,we can use udelay(1)*/
+ i = limit / (MMC_TIMEOUT_MS * 1000);
OMAP_HSMMC_WRITE(host-base, SYSCTL,
OMAP_HSMMC_READ(host-base, SYSCTL) | bit);
@@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct
omap_hsmmc_host *host,
* Monitor a 0-1 transition first
*/
if (mmc_slot(host).features HSMMC_HAS_UPDATED_RESET) {
- while ((!(OMAP_HSMMC_READ(host-base, SYSCTL) bit))
-(i++ limit))
- cpu_relax();
+ while (i--) {
+ if ((OMAP_HSMMC_READ(host-base, SYSCTL) bit))
+ break;
+ udelay(1);
+ }
}
- i = 0;
- while ((OMAP_HSMMC_READ(host-base, SYSCTL) bit)
- (i++ limit))
- cpu_relax();
+ i = limit / (MMC_TIMEOUT_MS * 1000);
+ while (i--) {
+ if (!(OMAP_HSMMC_READ(host-base, SYSCTL) bit))
+ break;
+ udealy(1);
+ }
if (OMAP_HSMMC_READ(host-base, SYSCTL) bit)
dev_err(mmc_dev(host-mmc),
--
1.8.1.2
Thanks!
Jianpeng MaN�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{雹f��{ay� �,j��f＂�h���z� �wア�
⒎�j:+v���w�j�m��赙zZ+�茛j��!�i
Re: [PATCH 4/9] dma: edma: Find missed events and issue them
On 07/31/2013 09:27 PM, Joel Fernandes wrote: On 07/31/2013 04:18 AM, Sekhar Nori wrote: On Wednesday 31 July 2013 10:19 AM, Joel Fernandes wrote: Hi Sekhar, On 07/30/2013 02:05 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: In an effort to move to using Scatter gather lists of any size with EDMA as discussed at [1] instead of placing limitations on the driver, we work through the limitations of the EDMAC hardware to find missed events and issue them. The sequence of events that require this are: For the scenario where MAX slots for an EDMA channel is 3: SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - Null The above SG list will have to be DMA'd in 2 sets: (1) SG1 - SG2 - SG3 - Null (2) SG4 - SG5 - SG6 - Null After (1) is succesfully transferred, the events from the MMC controller donot stop coming and are missed by the time we have setup the transfer for (2). So here, we catch the events missed as an error condition and issue them manually. Are you sure there wont be any effect of these missed events on the peripheral side. For example, wont McASP get into an underrun condition when it encounters a null PaRAM set? Even UART has to transmit to a But it will not encounter null PaRAM set because McASP uses contiguous buffers for transfer which are not scattered across physical memory. This can be accomplished with an SG of size 1. For such SGs, this patch series leaves it linked Dummy and does not link to Null set. Null set is only used for SG lists that are MAX_NR_SG in size such as those created for example by MMC and Crypto. particular baud so I guess it cannot wait like the way MMC/SD can. Existing driver have to wait anyway if they hit MAX SG limit today. If they don't want to wait, they would have allocated a contiguous block of memory and DMA that in one stretch so they don't lose any events, and in such cases we are not linking to Null. As long as DMA driver can advertize its MAX SG limit, peripherals can always work around that by limiting the number of sync events they generate so as to not having any of the events getting missed. With this series, I am worried that EDMA drivers is advertizing that it can handle any length SG list while not taking care of missing any events while doing so. This will break the assumptions that driver writers make. Sorry, just forgot to respond to not taking care of missing any events while doing so. Can you clarify this? DMA engine driver is taking care of missed events. Also- missing of events doesn't result in feedback to the peripheral. Peripheral sends even to DMA controller, event is missed. Peripheral doesn't know anything about what happened and is waiting for transfer from the DMA controller. Thanks, -Joel -- 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: [PATCHv7 1/2] drivers: spi: Add qspi flash controller
On Thursday 01 August 2013 12:09 AM, Trent Piepho wrote:
On Tue, Jul 30, 2013 at 10:47 PM, Sourav Poddarsourav.pod...@ti.com wrote:
Test details:
-
Tested this on dra7 board.
Test1: Ran mtd_stesstest for 4 iterations.
- All iterations went through without failure.
Test2: Use mtd utilities:
- flash_erase to erase the flash device
- nanddump to read data back.
- nandwrite to write to the data flash.
diff between the write and read data shows zero.
You've obviously never tested word lengths other than 8, because...
+static inline void ti_qspi_read_data(struct ti_qspi *qspi,
+ unsigned long reg, int wlen, u8 **rxbuf)
+{
+ switch (wlen) {
+ case 8:
+ **rxbuf = readb(qspi-base + reg);
+ dev_vdbg(qspi-dev, rx done, read %02x\n, *(*rxbuf));
+ *rxbuf += 1;
+ break;
+ case 16:
+ **rxbuf = readw(qspi-base + reg);
*rxbuf is a u8*. This means when you assign to **rxbuf the type of
the lvalue is u8. 8 bits. It does not matter what type the rvalue
is, u8, u16, or u32, the result will always be truncated to 8 bits.
May be, I can typecast the lvalue correspondingly before assigning.
IMHO, I toss the design of ti_qspi_read/write_data(). They look like
a function to read a generic register, yet it only makes sense with
QSPI_SPI_DATA_REG. Passing the pointer by address so it can be
incremented is ugly. And doesn't work at all, since the type of the
pointer isn't going to be the same for different word lengths. The
case statement inside the inner loop is inefficient. Each case is
largely duplicated code.
Yes, type of word length is not going to be the same. Hence, I kept it
as u8, then increment the pointer accordingly according to the case to
which it belongs.
Due to the different kind of variants used(read(b/w/l), each case has to
be replicated.
--
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 4/9] dma: edma: Find missed events and issue them
On 07/31/2013 09:27 PM, Joel Fernandes wrote: On 07/31/2013 04:18 AM, Sekhar Nori wrote: On Wednesday 31 July 2013 10:19 AM, Joel Fernandes wrote: Hi Sekhar, On 07/30/2013 02:05 AM, Sekhar Nori wrote: On Monday 29 July 2013 06:59 PM, Joel Fernandes wrote: In an effort to move to using Scatter gather lists of any size with EDMA as discussed at [1] instead of placing limitations on the driver, we work through the limitations of the EDMAC hardware to find missed events and issue them. The sequence of events that require this are: For the scenario where MAX slots for an EDMA channel is 3: SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - Null The above SG list will have to be DMA'd in 2 sets: (1) SG1 - SG2 - SG3 - Null (2) SG4 - SG5 - SG6 - Null After (1) is succesfully transferred, the events from the MMC controller donot stop coming and are missed by the time we have setup the transfer for (2). So here, we catch the events missed as an error condition and issue them manually. Are you sure there wont be any effect of these missed events on the peripheral side. For example, wont McASP get into an underrun condition when it encounters a null PaRAM set? Even UART has to transmit to a But it will not encounter null PaRAM set because McASP uses contiguous buffers for transfer which are not scattered across physical memory. This can be accomplished with an SG of size 1. For such SGs, this patch series leaves it linked Dummy and does not link to Null set. Null set is only used for SG lists that are MAX_NR_SG in size such as those created for example by MMC and Crypto. particular baud so I guess it cannot wait like the way MMC/SD can. Existing driver have to wait anyway if they hit MAX SG limit today. If they don't want to wait, they would have allocated a contiguous block of memory and DMA that in one stretch so they don't lose any events, and in such cases we are not linking to Null. As long as DMA driver can advertize its MAX SG limit, peripherals can always work around that by limiting the number of sync events they generate so as to not having any of the events getting missed. With this series, I am worried that EDMA drivers is advertizing that it can handle any length SG list while not taking care of missing any events while doing so. This will break the assumptions that driver writers make. This is already being done by some other DMA engine drivers ;). We can advertise more than we can handle at a time, that's the basis of this whole idea. I understand what you're saying but events are not something that have be serviced immediately, they can be queued etc and the actually transfer from the DMA controller can be delayed. As long as we don't miss the event we are fine which my series takes care off. So far I have tested this series on following modules in various configurations and have seen no issues: - Crypto AES - MMC/SD - SPI (128x160 display) Also, wont this lead to under-utilization of the peripheral bandwith? Meaning, MMC/SD is ready with data but cannot transfer because the DMA is waiting to be set-up. But it is waiting anyway even today. Currently based on MAX segs, MMC driver/subsystem will make SG list of size max_segs. Between these sessions of creating such smaller SG-lists, if for some reason the MMC controller is sending events, these will be lost anyway. But if MMC/SD driver knows how many events it should generate if it knows the MAX SG limit. So there should not be any missed events in current code. And I am not claiming that your solution is making matters worse. But its not making it much better as well. This is not true for crypto, the events are not deasserted and crypto continues to send events. This is what led to the don't trigger in Null patch where I'm setting the missed flag to avoid recursion. This can be used only for buffers that are contiguous in memory, not those that are scattered across memory. I was hinting at using the linking facility of EDMA to achieve this. Each PaRAM set has full 32-bit source and destination pointers so I see no reason why non-contiguous case cannot be handled. Lets say you need to transfer SG[0..6] on channel C. Now, PaRAM sets are typically 4 times the number of channels. In this case we use one DMA PaRAM set and two Link PaRAM sets per channel. P0 is the DMA PaRAM set and P1 and P2 are the Link sets. Initial setup: SG0 - SG1 - SG2 - SG3 - SG4 - SG5 - SG6 - NULL ^ ^ ^ | | | P0 - P1 - P2 - NULL P[0..2].TCINTEN = 1, so get an interrupt after each SG element completion. On each completion interrupt, hardware automatically copies the linked PaRAM set into the DMA PaRAM set so after SG0 is transferred out, the state of hardware is: SG1 - SG2 - SG3 - SG3 - SG6 - NULL ^ ^ | | P0,1P2 - NULL | ^ | | - SG1 transfer has already started by the time the TC interrupt is handled. As you
Re: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Thu, Aug 01, 2013 at 12:11:00AM +0300, Felipe Balbi wrote: that's the point, there is no known V3. Once it has, surely we will add such macros, but until then, we let the driver assume the highest known revision if it finds a register with an unknown revision. I am confused. The patch description says The new IP version has a minor changes and the offsets are same as the previous version, but you are saying there is no new version? Thanks, Richard -- 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: [net-next PATCH 1/1] drivers: net: cpsw: Add support for new CPSW IP version
On Wed, Jul 31, 2013 at 10:43:32PM +0300, Felipe Balbi wrote: right, now go check on the archives what Linus (and many others, for that matter) have said about version checking. If it's not the version you expect, you assume the latest. If you are talking about his essay about user space checking the kernel version, then that is another kettle of fish. Thanks, Richard -- 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
