[PATCH RFC 1/3] DT: add binding for MXS regulator
This patch adds the Device tree bindings for the Freescale MXS on-chip regulators. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../bindings/regulator/mxs-regulator.txt | 38 ++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt diff --git a/Documentation/devicetree/bindings/regulator/mxs-regulator.txt b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt new file mode 100644 index 000..c3226cc --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt @@ -0,0 +1,38 @@ +MXS regulators + +Required node properties: +- compatible: Should be fsl,soc-power, where soc is imx23 or imx28 +- reg: Offset and length of the register set for the device + +Required regulator properties: +- compatible: Must be fsl,mxs-regulator +- reg: Offset of the register set for the regulator +- mxs-max-reg-val: Maximum value of this register + +Optional regulator properties: +- mxs-default-microvolt: initial voltage of the regulator + +Any regulator property defined as part of the core regulator +binding, defined in regulator.txt, can also be used. + +Example: + + power: power@80044000 { + compatible = fsl,imx28-power; + #address-cells = 1; + #size-cells = 0; + reg = 0x80044000 0x2000; + + reg_vddio: regulator-vddio@80044060 { + reg = 0x80044060; + compatible = fsl,mxs-regulator; + regulator-name = vddio; + regulator-min-microvolt = 280; + regulator-max-microvolt = 360; + regulator-microvolt-offset = 8; + regulator-always-on; + mxs-max-reg-val = 0x10; + mxs-default-microvolt = 330; + }; + }; + -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 3/3] DT: ARM: mxs: enable regulator support for i.MX28
This patch enables the regulator support for i.MX28. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- arch/arm/boot/dts/imx28.dtsi | 50 +++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi index a95cc53..10c7cdf 100644 --- a/arch/arm/boot/dts/imx28.dtsi +++ b/arch/arm/boot/dts/imx28.dtsi @@ -979,8 +979,56 @@ }; power: power@80044000 { + compatible = fsl,imx28-power; + #address-cells = 1; + #size-cells = 0; reg = 0x80044000 0x2000; - status = disabled; + + /* + reg_curr: regulator-overall_current@80044010 { + reg = 0x80044010; + compatible = fsl,mxs-regulator; + regulator-name = overall_current; + regulator-min-microvolt = ; + regulator-max-microvolt = ; + regulator-min-microamp = ; + regulator-max-microamp = ; + regulator-always-on; + regulator-boot-on; + }; + */ + + reg_vddd: regulator-vddd@80044040 { + reg = 0x80044040; + compatible = fsl,mxs-regulator; + regulator-name = vddd; + regulator-min-microvolt = 80; + regulator-max-microvolt = 1575000; + regulator-always-on; + mxs-max-reg-val = 0x1f; + }; + + reg_vdda: regulator-vdda@80044050 { + reg = 0x80044050; + compatible = fsl,mxs-regulator; + regulator-name = vdda; + regulator-min-microvolt = 150; + regulator-max-microvolt = 2275000; + regulator-always-on; + mxs-max-reg-val = 0x1f; + }; + + reg_vddio: regulator-vddio@80044060 { + reg = 0x80044060; + compatible = fsl,mxs-regulator; + regulator-name = vddio; + regulator-min-microvolt = 280; + regulator-max-microvolt = 360; + regulator-microvolt-offset = 8; + regulator-always-on; + mxs-max-reg-val = 0x10; + mxs-default-microvolt = 330; + }; }; saif1: saif@80046000 { -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 0/3] ARM: regulator: add Freescale MXS regulator driver
This patch series draft should adds support for Freescale i.MX23, i.MX28 on-chip regulators. I'm new to regulator drivers and need early feedback for the driver's stage before i go in the wrong direction. The information about the i.MX28 regulators are from chapter 11 of the reference manual [1]. This driver based on the Freescale high level [2] and low level driver [3], but contains the following changes: * devictree support * code cleanup Please bear in mind that the code is not tested against real hardware and it's not expected to work. I'm not sure if it's correct, but these series based on the for-next branch of Mark Brown's regulator repository. Any comments about the code are welcome. [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf [2] - http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/regulator/mxs-regulator.c?h=imx_2.6.35_maintain [3] - http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx28/power.c?h=imx_2.6.35_maintain Stefan Wahren (3): DT: add binding for MXS regulator ARM: regulator: add Freescale MXS regulator driver DT: ARM: mxs: enable regulator support for i.MX28 .../bindings/regulator/mxs-regulator.txt | 38 ++ arch/arm/boot/dts/imx28.dtsi | 50 ++- drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/mxs-regulator.c | 411 + 5 files changed, 508 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt create mode 100644 drivers/regulator/mxs-regulator.c -- 1.8.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 2/3] ARM: regulator: add Freescale MXS regulator driver
This patch adds driver support for Freescale i.MX23, i.MX28 on-chip regulators. There are 4 voltage regulators: vddd, vdda, vddio, vddmem and 1 overall current regulator. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile| 1 + drivers/regulator/mxs-regulator.c | 411 ++ 3 files changed, 421 insertions(+) create mode 100644 drivers/regulator/mxs-regulator.c diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 7a3a061..e90e587 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -432,6 +432,15 @@ config REGULATOR_MC13892 Say y here to support the regulators found on the Freescale MC13892 PMIC. +config REGULATOR_MXS + tristate Freescale MXS on-chip regulators + depends on ARCH_MXS + default y if ARCH_MXS + help + Say y here to support Freescale MXS on-chip regulators. + It is recommended that this option be enabled on i.MX23, + i.MX28 platform. + config REGULATOR_PALMAS tristate TI Palmas PMIC Regulators depends on MFD_PALMAS diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 25271f8..0f5b66c 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o +obj-$(CONFIG_REGULATOR_MXS) += mxs-regulator.o obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o diff --git a/drivers/regulator/mxs-regulator.c b/drivers/regulator/mxs-regulator.c new file mode 100644 index 000..7d74518 --- /dev/null +++ b/drivers/regulator/mxs-regulator.c @@ -0,0 +1,411 @@ +/* + * Freescale STMP378X voltage regulators + * + * Embedded Alley Solutions, Inc sou...@embeddedalley.com + * + * Copyright (C) 2014 Stefan Wahren + * Copyright (C) 2010 Freescale Semiconductor, Inc. + * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved. + */ + +/* + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include linux/delay.h +#include linux/err.h +#include linux/init.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h +#include linux/of.h +#include linux/of_address.h +#include linux/platform_device.h +#include linux/regulator/driver.h +#include linux/regulator/machine.h +#include linux/regulator/of_regulator.h +#include linux/slab.h + +#define HW_POWER_VDDDCTRL (0x0040) +#define HW_POWER_VDDACTRL (0x0050) +#define HW_POWER_VDDIOCTRL (0x0060) +#define HW_POWER_STS (0x00c0) + +#define BM_POWER_STS_DC_OK (1 9) +#define BM_POWER_REG_MODE (1 17) +#define BM_POWER_LEVEL_TRG 0x1f + +#define MXS_REG5V_NOT_USB 0 +#define MXS_REG5V_IS_USB 1 + + +struct mxs_regulator { + struct regulator_desc rdesc; + struct regulator_init_data *initdata; + struct mxs_regulator *parent; + + spinlock_t lock; + wait_queue_head_t wait_q; + struct notifier_block nb; + + const char *name; + void __iomem *base_addr; + void __iomem *power_addr; + int mode; + int cur_uV; + int cur_uA; + int max_uA; + u32 max_reg_val; +}; + +static int mxs_set_voltage(struct regulator_dev *reg, int min_uV, int max_uV, + unsigned *selector) +{ + struct mxs_regulator *sreg = rdev_get_drvdata(reg); + struct regulation_constraints *con = sreg-initdata-constraints; + void __iomem *power_sts = sreg-power_addr + HW_POWER_STS; + u32 val, regs, i; + + pr_debug(%s: uv %d, min %d, max %d\n, __func__, + max_uV, con-min_uV, con-max_uV); + + if (max_uV con-min_uV || max_uV con-max_uV) + return -EINVAL; + + val = (max_uV - con-min_uV) * sreg-max_reg_val / + (con-max_uV - con-min_uV); + + regs = (__raw_readl(sreg-base_addr) ~BM_POWER_LEVEL_TRG); + pr_debug(%s: calculated val %d\n, __func__, val); + __raw_writel(val | regs, sreg-base_addr); + for (i = 20; i; i--) { + if (__raw_readl(power_sts) BM_POWER_STS_DC_OK) + break; + udelay(1); + } + + if (i) + goto out; + + __raw_writel(val | regs, sreg-base_addr); + for (i = 4; i; i--) { + if (__raw_readl(power_sts) BM_POWER_STS_DC_OK) + break
Re: [PATCH 1/2] DT: add binding for mxs regulator
Hi Mark, hi Fabio, Mark Rutland mark.rutl...@arm.com hat am 29. September 2014 um 15:23 geschrieben: I would prefer a top level node for the subsystem that is not a simple-bus. Give it a compatible string and a well-defined set of base properties (looks like you just need the reg for now). Match that and probe the child nodes as appropriate. Do we need a extra driver? Perhaps, but it's relatively simple to match on a compatible string and probe children if you just wantto start small for now. Mark. i would name the driver for the power subsystem mxs_power.c and use the compatibel string fsl,imx28-power for i.MX28. Now the question: where should i take? arch/arm/mach-mxs/ or drivers/power/ Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Regulator: Questions about constraints
Hi, currently i'm porting a regulator driver and i've two question about regulator constraints and DT binding: Should a regulator driver rely on DT regulator constraints or not? In case the constraints are not set by DT, what is the recommend behavior of a regulator driver? Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: add mxs regulator driver
Am 29.09.2014 um 19:13 schrieb Mark Brown: On Mon, Sep 29, 2014 at 08:38:51AM +0200, Stefan Wahren wrote: I'm searching for a good regulator implementation example. Does it apply to ti-abb-regulator.c and twl-regulator.c? Possibly. But bear in mind that it's important to understand the hardware you're trying to support. The question refer more to the devicetree binding and it's implementation. This really needs a comment to explain what on earth is going on here - the whole thing with writing the same thing twice with two delays is more than a little odd. It looks like the driver is trying to busy wait in cases where the change happens quickly but the comments about fast and normal mode make this unclear. The regulator driver polls for the DC_OK bit in the power status register. Quote for reference manual (p. 935): High when switching DC-DC converter control loop has stabilized after a voltage target change. The two loops comes from the different regulator modes (REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL). In REGULATOR_MODE_FAST the voltage steping is disabled and changing voltage should be fast. In REGULATOR_MODE_NORMAL voltage steping is enabled and it's take a while for reaching the target voltage. I don't think you've fully understood what the different modes mean here, that's not normally how a buck convertor works. The different modes would typically control the ability of the regulator to respond quickly to changes in load without drifting off regulation, fast mode makes the regulator less efficient but more responsive to load changes (probably marginally with modern regulators). It should have relatively little to do with the ability to ramp the voltage and certainly not on the scale there. Do you see more a problem with the two different loops or the redundant register write? Both. The code right now just looks really obscure. That leads me to the conclusion to drop both mode functions. My intention is to get the cpufreq-cpu0 aka cpufreq-dt working on i.MX28, not to build up the complete power system. @Fabio, @Shawn: What is your opinion? Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: add mxs regulator driver
Hi Mark, Mark Brown broo...@kernel.org hat am 29. September 2014 um 19:13 geschrieben: This really needs a comment to explain what on earth is going on here - the whole thing with writing the same thing twice with two delays is more than a little odd. It looks like the driver is trying to busy wait in cases where the change happens quickly but the comments about fast and normal mode make this unclear. The regulator driver polls for the DC_OK bit in the power status register. Quote for reference manual (p. 935): High when switching DC-DC converter control loop has stabilized after a voltage target change. The two loops comes from the different regulator modes (REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL). In REGULATOR_MODE_FAST the voltage steping is disabled and changing voltage should be fast. In REGULATOR_MODE_NORMAL voltage steping is enabled and it's take a while for reaching the target voltage. I don't think you've fully understood what the different modes mean here, that's not normally how a buck convertor works. The different modes would typically control the ability of the regulator to respond quickly to changes in load without drifting off regulation, fast mode makes the regulator less efficient but more responsive to load changes (probably marginally with modern regulators). It should have relatively little to do with the ability to ramp the voltage and certainly not on the scale there. thanks for your explanation. Is it better to let the core handles the ramp delay instead of set_voltage_sel with a busy wait? I've found that polling DC_OK is only reliable for increasing voltages. So i think about defining ramp delay in the regulator description. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 1/3] DT: add binding for MXS regulator
Hi Mark, Am 09.09.2014 19:59, schrieb Mark Rutland: On Sun, Sep 07, 2014 at 12:37:47PM +0100, Stefan Wahren wrote: This patch adds the Device tree bindings for the Freescale MXS on-chip regulators. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../bindings/regulator/mxs-regulator.txt | 38 ++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt diff --git a/Documentation/devicetree/bindings/regulator/mxs-regulator.txt b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt new file mode 100644 index 000..c3226cc --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt @@ -0,0 +1,38 @@ +MXS regulators + +Required node properties: +- compatible: Should be fsl,soc-power, where soc is imx23 or imx28 +- reg: Offset and length of the register set for the device We require #address-cells and #size-cells if the child nodes have reg entries. okay. + +Required regulator properties: +- compatible: Must be fsl,mxs-regulator +- reg: Offset of the register set for the regulator Is this the offset or the absolute physical address? The example seems to be absolute. You are right, the description is wrong. +- mxs-max-reg-val: Maximum value of this register What does this even mean? What's the format? Is this not implied by standard properties like regulator-max-microvolt? Unfortunately not as long the step size in microvolt isn't defined. The parameter defines the register value in hex corresponding to regulator-max-microvolt. step size = (regulator-max-microvolt - regulator-min-microvolt) / mxs-max-reg-val May be i missed or understand something wrong? +Optional regulator properties: +- mxs-default-microvolt: initial voltage of the regulator Why is this necessary? The original driver from Freescale set the MX28 regulator vddio to 3.38 V during registering. I think that is ugly to implement in the driver part, so i add a more general property in the devicetree binding. Mark. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 2/3] ARM: regulator: add Freescale MXS regulator driver
Hi, Am 09.09.2014 20:22, schrieb Mark Rutland: [...] + regs = (__raw_readl(sreg-base_addr) ~BM_POWER_LEVEL_TRG); I suspect you should be using the *_relaxed accessors rather than the __raw_* accessors. [...] +static int mxs_regulator_probe(struct platform_device *pdev) +{ + struct device *dev = pdev-dev; + struct device_node *np = dev-of_node; + struct device_node *parent; + struct regulator_desc *rdesc; + struct regulator_dev *rdev; + struct mxs_regulator *sreg; + struct regulator_init_data *initdata; + struct regulation_constraints *con; + struct regulator_config config = { }; + void __iomem *base_addr = NULL; + void __iomem *power_addr = NULL; + u64 regaddr64 = 0; + const u32 *regaddr_p; + u32 val = 0; + int ret; + + if (!np) { + dev_err(dev, missing device tree\n); + return -EINVAL; + } + + /* get device base address */ + base_addr = of_iomap(np, 0); + if (!base_addr) + return -ENXIO; + + parent = of_get_parent(np); + if (!parent) + return -ENXIO; Leak of the (successfully mapped) base_addr. + + power_addr = of_iomap(parent, 0); + if (!power_addr) + return -ENXIO; Leak of base_addr and dangling refcount on parent. These apply to all subsequent returns. + + regaddr_p = of_get_address(np, 0, NULL, NULL); of_get_address returns a __be32*, not a u32*, so sparse will be very unhappy here... + if (regaddr_p) + regaddr64 = of_translate_address(np, regaddr_p); ...and as of_translate_address returns a u64 you'll need a separate variable for the input and output. + + if (!regaddr64) { + dev_err(dev, no or invalid reg property set\n); + return -EINVAL; + } + + initdata = of_get_regulator_init_data(dev, np); + if (!initdata) + return -EINVAL; + + ret = of_property_read_u32(np, mxs-max-reg-val, + val); + if (!val) { + dev_err(dev, no or invalid mxs-max-reg-val property set\n); + return ret; + } + + dev_info(dev, regulator found\n); + + sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL); + if (!sreg) + return -ENOMEM; + sreg-initdata = initdata; + sreg-name = of_get_property(np, regulator-name, NULL); I'm not keen on using of_get_property here. We have no idea if regulator-name is even a string (it should be, but we have no guarantee). Better using of_property_read_string? + sreg-cur_uA = 0; + sreg-cur_uV = 0; + sreg-base_addr = base_addr; + sreg-power_addr = power_addr; + init_waitqueue_head(sreg-wait_q); + spin_lock_init(sreg-lock); + sreg-max_reg_val = val; + + rdesc = sreg-rdesc; + rdesc-name = sreg-name; + rdesc-owner = THIS_MODULE; + rdesc-ops = mxs_rops; + + if (strcmp(rdesc-name, overall_current) == 0) + rdesc-type = REGULATOR_CURRENT; + else + rdesc-type = REGULATOR_VOLTAGE; Wouldn't it make more sense to explicitly match the names you expect? Okay, i make regulator-name a required property and use a white list of all possible regulators. + con = initdata-constraints; + rdesc-n_voltages = sreg-max_reg_val; + rdesc-min_uV = con-min_uV; + rdesc-uV_step = (con-max_uV - con-min_uV) / sreg-max_reg_val; + rdesc-linear_min_sel = 0; + rdesc-vsel_reg = regaddr64; + rdesc-vsel_mask = BM_POWER_LEVEL_TRG; + + config.dev = pdev-dev; + config.init_data = initdata; + config.driver_data = sreg; + config.of_node = np; + + pr_debug(probing regulator %s %s %d\n, + sreg-name, + rdesc-name, + pdev-id); Aren't those two names always the same per the code above? Sure, i will fix that. + + /* register regulator */ + rdev = devm_regulator_register(dev, rdesc, config); + + if (IS_ERR(rdev)) { + dev_err(pdev-dev, failed to register %s\n, + rdesc-name); + return PTR_ERR(rdev); + } + + if (sreg-max_uA) { + struct regulator *regu; + + regu = regulator_get(NULL, sreg-name); + sreg-nb.notifier_call = reg_callback; + regulator_register_notifier(regu, sreg-nb); + } + + platform_set_drvdata(pdev, rdev); + + of_property_read_u32(np, mxs-default-microvolt, + val); + + if (val) + mxs_set_voltage(rdev, val, val, NULL); As I mentioned in my comments on the binding, I'd like to know why this is necessary and if it is why it shouldn't be a standardised property. From my understanding the standardised properties only defines
Re: [PATCH RFC 2/3] ARM: regulator: add Freescale MXS regulator driver
Hi, Am 10.09.2014 16:18, schrieb Mark Rutland: [...] + of_property_read_u32(np, mxs-default-microvolt, + val); + + if (val) + mxs_set_voltage(rdev, val, val, NULL); As I mentioned in my comments on the binding, I'd like to know why this is necessary and if it is why it shouldn't be a standardised property. From my understanding the standardised properties only defines a range, but no default state of the regulators. If the initialization from the bootloader or a hardcoded initialization in the driver is okay then the property is not necessary. Sure. My questions was why it is necessary to preconfigure the regulators at all rather than why it is necessary to do so in this manner. Mark. sorry i don't have a clue. In the original code there isn't a comment about the reason. Currently there is no init of the vddio regulator by the kernel and everything works fine. @Fabio: Do you have any doubts? Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 2/3] ARM: regulator: add Freescale MXS regulator driver
Hi Mark, Am 10.09.2014 17:13, schrieb Mark Brown: On Wed, Sep 10, 2014 at 03:18:53PM +0100, Mark Rutland wrote: On Tue, Sep 09, 2014 at 08:17:17PM +0100, Stefan Wahren wrote: Ugh, this looks like it might be a regulator driver but since the subject line was ARM: I deleted it unread - if your changelog looks different to all the other changelogs in the subsystem it probably needs changing. sorry about the confusion, i will remove ARM in the next version. Changelog? I didn't send a changelog because it was my first version. Should i resend this version only to you? + sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL); + if (!sreg) + return -ENOMEM; + sreg-initdata = initdata; + sreg-name = of_get_property(np, regulator-name, NULL); I'm not keen on using of_get_property here. We have no idea if regulator-name is even a string (it should be, but we have no guarantee). Better using of_property_read_string? Yes. That will check the value is NUL-terminated, at least. Or just remove the property entirely... without having seen the bindings if we're specifying the name of the device via the device tree something seems wrong. BR Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 2/3] ARM: regulator: add Freescale MXS regulator driver
Hi Fabio, Am 10.09.2014 20:54, schrieb Fabio Estevam: Hi Stefan, On Wed, Sep 10, 2014 at 2:32 PM, Stefan Wahren i...@lategoodbye.de wrote: Hi Mark, Am 10.09.2014 17:13, schrieb Mark Brown: On Wed, Sep 10, 2014 at 03:18:53PM +0100, Mark Rutland wrote: On Tue, Sep 09, 2014 at 08:17:17PM +0100, Stefan Wahren wrote: Ugh, this looks like it might be a regulator driver but since the subject line was ARM: I deleted it unread - if your changelog looks different to all the other changelogs in the subsystem it probably needs changing. sorry about the confusion, i will remove ARM in the next version. Changelog? I didn't send a changelog because it was my first version. Should i resend this version only to you? In the cover letter of this RFC series you mentioned that this has not been tested on real hardware. What about sending a new version of this series (with the RFC prefix removed and with Mark Rutland's suggestion implemented) tested on a mx28 board and also with the Subject of the regulator patch changed to 'regulator: add support for mxs regulator with Mark Brown on Cc? that's the same idea i had about the first real version of my patch. Unfortunately i can do the porting only in my spare time. So i try to avoid with the RFC series the situation, that i spend many days in development and testing, but after it the regulator guys says it went in the complete wrong direction. The advice about the anatop regulator was helpful, but there is still some unsureness. I will try to get a Duckbill, so i can do testing at home. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] regulator: add support for mxs regulator
This patch series adds support for Freescale i.MX23, i.xM28 on-chip regulators: vddd, vdda, vddio This driver based on the Freescale high level [1] and low level driver [2], but contains the following changes: * devicetree support * fix for regulator modes * drop support for overall_current and brown out * replace udelay() with schedule() * code cleanup The code has been tested on a I2SE Duckbill. [1] - http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/regulator/mxs-regulator.c?h=imx_2.6.35_maintain [2] - http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx28/power.c?h=imx_2.6.35_maintain Stefan Wahren (2): DT: add binding for mxs regulator regulator: add mxs regulator driver .../bindings/regulator/mxs-regulator.txt | 36 ++ drivers/regulator/Kconfig | 11 + drivers/regulator/Makefile |1 + drivers/regulator/mxs-regulator.c | 395 4 files changed, 443 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt create mode 100644 drivers/regulator/mxs-regulator.c -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] regulator: add mxs regulator driver
This patch adds driver support for Freescale i.MX23, i.MX28 on-chip regulators. The driver supports the following regulators: vddd, vdda and vddio. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/regulator/Kconfig | 11 ++ drivers/regulator/Makefile|1 + drivers/regulator/mxs-regulator.c | 395 + 3 files changed, 407 insertions(+) create mode 100644 drivers/regulator/mxs-regulator.c diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index a353011..f353a2b 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -433,6 +433,17 @@ config REGULATOR_MC13892 Say y here to support the regulators found on the Freescale MC13892 PMIC. +config REGULATOR_MXS + tristate Freescale MXS on-chip regulators + depends on ARCH_MXS + default y if ARCH_MXS + help + Say y here to support Freescale MXS on-chip regulators. + + The driver currently support the following voltage regulators: + vddd, vdda and vddio. It is recommended that this option be + enabled on i.MX23, i.MX28 platform. + config REGULATOR_PALMAS tristate TI Palmas PMIC Regulators depends on MFD_PALMAS diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 25271f8..0f5b66c 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_REGULATOR_MAX77802) += max77802.o obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o +obj-$(CONFIG_REGULATOR_MXS) += mxs-regulator.o obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o diff --git a/drivers/regulator/mxs-regulator.c b/drivers/regulator/mxs-regulator.c new file mode 100644 index 000..63c09cc --- /dev/null +++ b/drivers/regulator/mxs-regulator.c @@ -0,0 +1,395 @@ +/* + * Freescale STMP378X voltage regulators + * + * Embedded Alley Solutions, Inc sou...@embeddedalley.com + * + * Copyright (C) 2014 Stefan Wahren + * Copyright (C) 2010 Freescale Semiconductor, Inc. + * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved. + */ + +/* + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include linux/delay.h +#include linux/err.h +#include linux/init.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h +#include linux/of.h +#include linux/of_address.h +#include linux/platform_device.h +#include linux/regulator/driver.h +#include linux/regulator/machine.h +#include linux/regulator/of_regulator.h +#include linux/slab.h + +#define HW_POWER_STS (0x00c0) + +#define BM_POWER_STS_DC_OK BIT(9) + +#define MXS_VDDIO 1 +#define MXS_VDDA 2 +#define MXS_VDDD 3 + +struct mxs_regulator { + struct regulator_desc rdesc; + struct regulator_init_data *initdata; + + const char *name; + void __iomem *base_addr; + void __iomem *power_addr; + unsigned int mode_mask; +}; + +static int mxs_set_voltage(struct regulator_dev *reg, int min_uV, int max_uV, + unsigned *selector) +{ + struct mxs_regulator *sreg = rdev_get_drvdata(reg); + struct regulation_constraints *con = sreg-initdata-constraints; + void __iomem *power_sts = sreg-power_addr + HW_POWER_STS; + unsigned long start; + u32 val, regs, i; + + pr_debug(%s: min_uV %d, max_uV %d, min %d, max %d\n, __func__, +min_uV, max_uV, con-min_uV, con-max_uV); + + if (max_uV con-min_uV || max_uV con-max_uV) + return -EINVAL; + + val = (max_uV - con-min_uV) * sreg-rdesc.n_voltages / + (con-max_uV - con-min_uV); + + regs = (readl(sreg-base_addr) ~sreg-rdesc.vsel_mask); + + pr_debug(%s: %s calculated val %d\n, __func__, sreg-name, val); + + writel(val | regs, sreg-base_addr); + for (i = 20; i; i--) { + /* delay for fast mode */ + if (readl(power_sts) BM_POWER_STS_DC_OK) + return 0; + + udelay(1); + } + + writel(val | regs, sreg-base_addr); + start = jiffies; + while (1) { + /* delay for normal mode */ + if (readl(power_sts) BM_POWER_STS_DC_OK) + return 0; + + if (time_after(jiffies, start + msecs_to_jiffies(80))) + break; + + schedule(); + } + + return -ETIMEDOUT; +} + + +static int
[PATCH 1/2] DT: add binding for mxs regulator
This patch adds the Device tree bindings for the Freescale MXS on-chip regulators. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../bindings/regulator/mxs-regulator.txt | 36 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt diff --git a/Documentation/devicetree/bindings/regulator/mxs-regulator.txt b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt new file mode 100644 index 000..e3133a4 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt @@ -0,0 +1,36 @@ +MXS regulators + +Required node properties: +- compatible: Should be simple-bus +- #address-cells: Number of cells required to define regulator register, + must be 1 +- #size-cells: Number of cells required to define register size, must be 1 +- reg: Absolute physical address and size of the register set for the device + +Required regulator properties: +- compatible: Must be fsl,mxs-regulator +- reg: Absolute physical address of the register set for the regulator + +Any regulator property defined as part of the core regulator +binding, defined in regulator.txt, can also be used. + +Example: + + power: power@80044000 { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 1; + reg = 0x80044000 0x2000; + ranges; + + reg_vddd: regulator@80044040 { + reg = 0x80044040 0x10; + compatible = fsl,mxs-regulator; + regulator-name = vddd; + regulator-min-microvolt = 80; + regulator-max-microvolt = 1575000; + regulator-boot-on; + vddd-supply = reg_vdda; + }; + }; + -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] DT: add binding for mxs regulator
Hi Mark, Am 28.09.2014 um 12:22 schrieb Mark Brown: On Sat, Sep 27, 2014 at 12:59:47AM +, Stefan Wahren wrote: This patch adds the Device tree bindings for the Freescale MXS on-chip regulators. Use subject lines matching the style for the subsystem. sorry i'm not sure what's wrong with the subject lines. Did you expect [PATCH 1/2] regulator: add binding for mxs regulator? +Required regulator properties: +- compatible: Must be fsl,mxs-regulator +- reg: Absolute physical address of the register set for the regulator + +Any regulator property defined as part of the core regulator +binding, defined in regulator.txt, can also be used. While this should be using compatibles to identify which regulator is being supported note that the binding doesn't document the fact that the code makes regulator-name mandatory or what values are required. Is the following better? - fsl,mxs-regulator-vddd - fsl,mxs-regulator-vdda - fsl,mxs-regulator-vddio Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] regulator: add mxs regulator driver
Hi Mark, thanks for your comments. Now it looks to me, that i try to reinvent the wheel. I'm searching for a good regulator implementation example. Does it apply to ti-abb-regulator.c and twl-regulator.c? Am 28.09.2014 um 12:16 schrieb Mark Brown: On Sat, Sep 27, 2014 at 12:59:48AM +, Stefan Wahren wrote: +pr_debug(%s: min_uV %d, max_uV %d, min %d, max %d\n, __func__, + min_uV, max_uV, con-min_uV, con-max_uV); + +if (max_uV con-min_uV || max_uV con-max_uV) +return -EINVAL; This is replicating checks done by the core. +val = (max_uV - con-min_uV) * sreg-rdesc.n_voltages / +(con-max_uV - con-min_uV); Drivers should never look at their constraints, they should let the core do that and just do what they're asked. In this case it is probably best to implement a get_voltage_sel() operation and have the conversion to voltage done by regulator_map_voltage_linear(), this will both make the code look better and mean the driver gets the benefit of all the error checking done by the core. Okay, i will do that. For the list_voltage operation regulator_list_voltage_linear() should be the perfect candidate. +writel(val | regs, sreg-base_addr); +for (i = 20; i; i--) { +/* delay for fast mode */ +if (readl(power_sts) BM_POWER_STS_DC_OK) +return 0; + +udelay(1); +} + +writel(val | regs, sreg-base_addr); +start = jiffies; +while (1) { +/* delay for normal mode */ +if (readl(power_sts) BM_POWER_STS_DC_OK) +return 0; This really needs a comment to explain what on earth is going on here - the whole thing with writing the same thing twice with two delays is more than a little odd. It looks like the driver is trying to busy wait in cases where the change happens quickly but the comments about fast and normal mode make this unclear. The regulator driver polls for the DC_OK bit in the power status register. Quote for reference manual (p. 935): High when switching DC-DC converter control loop has stabilized after a voltage target change. The two loops comes from the different regulator modes (REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL). In REGULATOR_MODE_FAST the voltage steping is disabled and changing voltage should be fast. In REGULATOR_MODE_NORMAL voltage steping is enabled and it's take a while for reaching the target voltage. Do you see more a problem with the two different loops or the redundant register write? Is it acceptable that the function blocks here? +pr_debug(%s: %s register val %d\n, __func__, sreg-name, val); + +switch (sreg-rdesc.id) { +case MXS_VDDA: +val = 16; +break; +case MXS_VDDD: +val = 20; +break; +} + +return val ? 1 : 0; +} This seems buggy - it'll always return true for MXS_VDDD if MXS_VDDA enabled won't it? Shame on me, i forgot to remove this function. mxs_is_enabled is never used. +static unsigned int mxs_get_mode(struct regulator_dev *reg) +{ +struct mxs_regulator *sreg = rdev_get_drvdata(reg); +u32 val = readl(sreg-base_addr) sreg-mode_mask; + +return val ? REGULATOR_MODE_FAST : REGULATOR_MODE_NORMAL; +} Please try to avoid the ternery operator, it does nothing for legibility. if (readl(sreg-base_addr) sreg-mode_mask) return REGULATOR_MODE_FAST; return REGULATOR_MODE_NORMAL; Better? +if (of_property_read_string(np, regulator-name, name)) { +dev_err(dev, missing property regulator-name\n); +return -EINVAL; +} Use different compatibles to identify the regulators, regulator-name should never be mandatory. I will remove this and use the compatibles. +switch (sreg-rdesc.id) { +case MXS_VDDIO: +sreg-mode_mask = BIT(17); +break; +case MXS_VDDA: +sreg-mode_mask = BIT(18); +break; +case MXS_VDDD: +sreg-mode_mask = BIT(22); +break; +} Why is this not looked up from the data structure like the rest of the data? I was a little bit confused why there wasn't a mode_mask in the struct regulator_desc. I will do it like the ti-abb-regulator in the matching table. +dev_info(dev, %s found\n, name); Don't add noise like this to the boot log, it provides no useful information. Okay, i will remove this. +regulator_unregister(rdev); +iounmap(power_addr); +iounmap(base_addr); Use devm_ versions of functions. As a result the remove function for the drivers becomes unnecessary. Am i right? +static int __init mxs_regulator_init(void) +{ +return platform_driver_register(mxs_regulator_driver); +} +postcore_initcall(mxs_regulator_init); module_platform_driver(). I wasn't sure because of the postcore stuff. I will fix it. Best regards Stefan -- To unsubscribe from this list: send the line
Re: [PATCH 1/2] DT: add binding for mxs regulator
Hi Mark, Am 29.09.2014 um 13:09 schrieb Mark Rutland: On Sat, Sep 27, 2014 at 01:59:47AM +0100, Stefan Wahren wrote: This patch adds the Device tree bindings for the Freescale MXS on-chip regulators. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../bindings/regulator/mxs-regulator.txt | 36 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt diff --git a/Documentation/devicetree/bindings/regulator/mxs-regulator.txt b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt new file mode 100644 index 000..e3133a4 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt @@ -0,0 +1,36 @@ +MXS regulators + +Required node properties: +- compatible: Should be simple-bus This does not look like an appropriate use of simple-bus. Why do you want the parent node to be a simple-bus? the current parent node in imx28.dtsi looks like a placeholder for the power sub system: power: power@80044000 { reg = 0x80044000 0x2000; status = disabled; }; I want to trigger the probing of the child nodes (regulators) without writing a driver for the complete power sub system. The simple-bus avoids that. Do we need a extra driver? +- #address-cells: Number of cells required to define regulator register, + must be 1 +- #size-cells: Number of cells required to define register size, must be 1 Why must this be the case, given that the child node expects an absolute physical address? I need a property to define the control register for the regulators without defining vendor specific properties like fsl,mxs-control-reg or something. What's wrong with #address-cells = 2, for example? Nothing +- reg: Absolute physical address and size of the register set for the device Why is this here _and_ in the child node(s)? The parent of the power node is also a simple bus. I use this to calculate the power status register per offset. What is the difference between this node and its children? The parent node represent the power sub system and the regulators are part of this sub system. Can there be more than one sub-node? In the i.MX28 are at least 4 voltage regulators, 1 current regulator and many more. At first, the driver should implement only 3 voltage regulators (vddd, vdda, vddio). Mark. Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] DT: add binding for mxs regulator
Hi, Am 29.09.2014 um 14:41 schrieb Mark Rutland: Well, the simple-bus will cause the children to be probed. But it looks like you care about properties of the parent. I don't think that simple-bus is appropriate because it's not being handled as a transparent bridge from the PoV of the children. actually i need the address of the power status register. In this version i get the base address from the parent node add an offset. Do you prefer to define the address of the power status register like a second address cell: reg_vddd: regulator@80044040 { reg = 0x80044040 0x10 0x800440c0 0x01; ... }; or do i need special properties like this: reg_vddd: regulator@80044040 { reg = 0x80044040 0x10; fsl,mxs-status-reg = 0x800440c0; ... }; Do we need a extra driver? Perhaps, but it's relatively simple to match on a compatible string and probe children if you just wantto start small for now. Okay. Would be great if someone has a good example. At first, i thought of power/anatop. +- #address-cells: Number of cells required to define regulator register, + must be 1 +- #size-cells: Number of cells required to define register size, must be 1 Why must this be the case, given that the child node expects an absolute physical address? I need a property to define the control register for the regulators without defining vendor specific properties like fsl,mxs-control-reg or something. You misunderstand me. I was querying the must be 1 rather than the proeprties themselves. What's wrong with #address-cells = 2, for example? Nothing Then we shouldn't specify must be 1, no? Right, must be at least 1. +- reg: Absolute physical address and size of the register set for the device Why is this here _and_ in the child node(s)? The parent of the power node is also a simple bus. I use this to calculate the power status register per offset. What is the difference between this node and its children? The parent node represent the power sub system and the regulators are part of this sub system. Can there be more than one sub-node? In the i.MX28 are at least 4 voltage regulators, 1 current regulator and many more. At first, the driver should implement only 3 voltage regulators (vddd, vdda, vddio). Ok. I think you need a binding for the power subsystem, and a trivial driver that can match on that and probe the child regulators. Are there components other than voltage or current regulators in the sub system? Yes, according to the reference manual there is a dc-dc converter, a battery charger, battery monitor, ... In short a lot of developing time ;-) Mark. Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v3 02/18] of: Add vendor prefix for Himax Technologies Inc.
Hi Liu, Liu Ying ying@freescale.com hat am 23. Dezember 2014 um 04:46 geschrieben: Signed-off-by: Liu Ying ying@freescale.com --- v2-v3: * None. v1-v2: * None. Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 78efebb..3cee528 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -69,6 +69,7 @@ hannstar HannStar Display Corporation haoyu Haoyu Microelectronic Co. Ltd. hisilicon Hisilicon Limited. hit Hitachi Ltd. +himax Himax Technologies, Inc. please fix the ordering. himax comes before hisilicon. Thanks Stefan honeywell Honeywell hp Hewlett Packard i2se I2SE GmbH -- 2.1.0 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v3 11/18] drm/bridge: Add Synopsys DesignWare MIPI DSI host controller driver
Hi Liu, Liu Ying ying@freescale.com hat am 23. Dezember 2014 um 04:46 geschrieben: This patch adds Synopsys DesignWare MIPI DSI host controller driver support. Currently, the driver supports the burst with sync pulses mode only. Signed-off-by: Liu Ying ying@freescale.com --- v2-v3: * Newly introduced in v3 to address Andy Yan's comment. This is based on the i.MX MIPI DSI driver in v2. To make the Synopsys DesignWare MIPI DSI host controller driver less platform-dependant, this patch places it at the drm/bridge directory as a DRM bridge driver. .../devicetree/bindings/drm/bridge/dw_mipi_dsi.txt | 72 ++ please take care of [1] and send the Documentation part as a separate patch. This should also apply to all the other patches from your series. Thanks Stefan [1] - http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/submitting-patches.txt -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] clk: mxs: Fix invalid 32-bit access to frac registers
According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. This patch fixes the erroneous 32-bit access to these registers and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested only with a i.MX28 board, because i don't have access to an i.MX23 board. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- Changes in V2: - use relaxed access operations in clk-ref drivers/clk/mxs/clk-imx23.c | 11 --- drivers/clk/mxs/clk-imx28.c | 19 +-- drivers/clk/mxs/clk-ref.c | 19 ++- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..a084566 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,11 +46,13 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO3 static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -72,9 +74,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. +* According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + frac = readb_relaxed(FRAC + FRAC_IO); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC + FRAC_IO); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..c541377 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -118,11 +120,16 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + frac = readb_relaxed(FRAC0 + FRAC0_IO0); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); + frac = readb_relaxed(FRAC0 + FRAC0_IO1); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..ad3851c 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate; - u8 frac = (readl_relaxed(ref-reg) (ref-idx * 8)) 0x3f; + u8 frac = readb_relaxed(ref-reg + ref-idx) 0x3f; tmp *= 18; do_div(tmp, frac); @@ -93,8 +95,7 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, struct clk_ref *ref = to_clk_ref(hw); unsigned long flags; u64 tmp = parent_rate; - u32 val
Re: [PATCHv3 9/9] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
Hi Lothar, Lothar Waßmann l...@karo-electronics.de hat am 28. Oktober 2014 um 14:23 geschrieben: commit 1b7bde6d659d (net: fec: implement rx_copybreak to improve rx performance) introduced a regression for i.MX28. The swap_buffer() function doing the endian conversion of the received data on i.MX28 may access memory beyond the actual packet size in the DMA buffer. fec_enet_copybreak() does not copy those bytes, so that the last bytes of a packet may be filled with invalid data after swapping. This will likely lead to checksum errors on received packets. E.g. when trying to mount an NFS rootfs: UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36 i think i experience the same problem with 3.18-rc2 on my mx28 board. I get strange warnings about unexpected bytes from ping on my mx28 board and ping to my mx28 board fails because of no response. After applying the complete patch series these problems disappear. I'm looking forward to see V4. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. This patch fixes the erroneous 32-bit access to these registers. The changes has been tested only with a i.MX28 board, because i don't have access to an i.MX23 board. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/clk/mxs/clk-imx23.c |8 +--- drivers/clk/mxs/clk-imx28.c | 14 -- drivers/clk/mxs/clk-ref.c | 19 ++- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..371ba03 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,7 +46,8 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO3 static void __init clk_misc_init(void) { @@ -72,9 +73,10 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. +* According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + writeb_relaxed(0x3f, FRAC + FRAC_IO + CLR); + writeb_relaxed(30, FRAC + FRAC_IO + SET); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..3eae119 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -118,11 +119,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO0 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO0 + SET); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO1 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO1 + SET); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..ad3851c 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate; - u8 frac = (readl_relaxed(ref-reg) (ref-idx * 8)) 0x3f; + u8 frac = readb_relaxed(ref-reg + ref-idx) 0x3f; tmp *= 18; do_div(tmp, frac); @@ -93,8 +95,7 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, struct clk_ref *ref = to_clk_ref(hw); unsigned long flags; u64 tmp = parent_rate; - u32 val; - u8 frac, shift = ref-idx * 8; + u8 frac, val; tmp = tmp * 18 + rate / 2; do_div(tmp, rate); @@ -107,10 +108,10 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, spin_lock_irqsave(mxs_lock, flags); - val = readl_relaxed(ref-reg); - val = ~(0x3f shift); - val |= frac shift; - writel_relaxed(val, ref-reg); + val = readb_relaxed(ref-reg + ref-idx); + val = ~0x3f; + val |= frac; + writeb_relaxed(val, ref-reg + ref-idx); spin_unlock_irqrestore(mxs_lock, flags); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord
Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Marek, Marek Vasut ma...@denx.de hat am 14. Dezember 2014 um 17:12 geschrieben: static void __iomem *digctrl; #define DIGCTRL digctrl @@ -118,11 +119,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. + * According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO0 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO0 + SET); + writeb_relaxed(0x3f, FRAC0 + FRAC0_IO1 + CLR); + writeb_relaxed(30, FRAC0 + FRAC0_IO1 + SET); This used to be a R-M-W sequence, but now it's changed to multiple writes. This changes the behavior and seeing you use the CLR register, I am worried this might be prone to clock glitches. What do you think please ? you are right. I adapt the imx23 init to the imx28 to make code simple. But it would be better to avoid glitches. I hope it's okay for this bugfix to introduce a R-M-W sequence for the imx23 init. So it's consequent. [...] Also, it might be a good idea to zap the 0x3f mask and use HEX and DEC numbers consistently, but this is an idea for another patch. Yes. Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. But i will have access to that hardware tomorrow. Thanks Stefan Best regards, ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
Hi, Am 14.12.2014 um 20:19 schrieb Marek Vasut: On Sunday, December 14, 2014 at 06:16:17 PM, Stefan Wahren wrote: [...] Also, it might be a good idea to zap the 0x3f mask and use HEX and DEC numbers consistently, but this is an idea for another patch. Yes. Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. But i will have access to that hardware tomorrow. Which issue would that be please ? i've posted the issue in the Freescale Community [1]. What are the symptoms ? Bits from the SPI slave are misinterpreted. BR Stefan [1] - https://community.freescale.com/thread/310434 Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Marek, Am 17.12.2014 um 17:00 schrieb Marek Vasut: On Wednesday, December 17, 2014 at 08:58:23 AM, Stefan Wahren wrote: Hi Fabio, Am 17.12.2014 um 03:44 schrieb Fabio Estevam: Hi Stefan, On Sun, Dec 14, 2014 at 3:16 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. Does this patch fix the SPI issue? unfortunately not. If the 3.19-rc1 has been released, i send the fixed patch. I've the theory that's an initialization issue. I've never heard of SPI problems from the U-Boot users and we are using imx-bootlets. I'll try to test it with U-Boot. Please keep me in the loop, I'd be interested in what you find. Thanks! Best regards, Marek Vasut i tested it with U-Boot, but the issue still exists. If there is any progress, i'll inform you. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clk: mxs: Fix invalid 32-bit access to frac registers
According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. This patch fixes the erroneous 32-bit access to these registers and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested only with a i.MX28 board, because i don't have access to an i.MX23 board. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/clk/mxs/clk-imx23.c | 11 --- drivers/clk/mxs/clk-imx28.c | 19 +-- drivers/clk/mxs/clk-ref.c | 19 ++- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..a084566 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,11 +46,13 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO3 static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -72,9 +74,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. +* According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + frac = readb_relaxed(FRAC + FRAC_IO); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC + FRAC_IO); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..c541377 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -118,11 +120,16 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + frac = readb_relaxed(FRAC0 + FRAC0_IO0); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); + frac = readb_relaxed(FRAC0 + FRAC0_IO1); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..bdecec1 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb(BF_CLKGATE, ref-reg + ref-idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb(BF_CLKGATE, ref-reg + ref-idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate; - u8 frac = (readl_relaxed(ref-reg) (ref-idx * 8)) 0x3f; + u8 frac = readb(ref-reg + ref-idx) 0x3f; tmp *= 18; do_div(tmp, frac); @@ -93,8 +95,7 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, struct clk_ref *ref = to_clk_ref(hw); unsigned long flags; u64 tmp = parent_rate; - u32 val; - u8 frac, shift = ref-idx * 8; + u8 frac, val; tmp = tmp
Re: [PATCH] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Marek, Marek Vasut ma...@denx.de hat am 21. Dezember 2014 um 22:50 geschrieben: On Sunday, December 21, 2014 at 02:46:39 PM, Stefan Wahren wrote: Hi! [...] diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..bdecec1 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb(BF_CLKGATE, ref-reg + ref-idx + CLR); Should this be writeb_relaxed() maybe ? return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb(BF_CLKGATE, ref-reg + ref-idx + SET); Same here and all around the place ? okay, i will fix it in V2. Other than that, it looks pretty OK :) Fine Best regards, Marek Vasut Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Am 17.12.2014 um 03:44 schrieb Fabio Estevam: Hi Stefan, On Sun, Dec 14, 2014 at 3:16 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Btw i hope this patch also fixes a SPI communication issue with our hardware which forces us to bypass ref_io1 for ssp2. Does this patch fix the SPI issue? unfortunately not. If the 3.19-rc1 has been released, i send the fixed patch. I've the theory that's an initialization issue. I've never heard of SPI problems from the U-Boot users and we are using imx-bootlets. I'll try to test it with U-Boot. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] rockchip: efuse: add driver to support rk3288 efuse
Hi Jianqun, Jianqun Xu jay...@rock-chips.com hat am 1. Dezember 2014 um 08:34 geschrieben: In RK3288, there are two eFuse. One is organized as 32bits by 8 one-time programmable electrical fuses with random access interface, and the other is organized as 32bits by 32 one-time programmable electrical fuses. Jianqun Xu (2): rockchip: efuse: add documentation for rk3288 efuse driver rockchip: efuse: add efuse driver for rk3288 efuse .../bindings/fuse/rockchip,rk3288-efuse.txt | 14 ++ arch/arm/mach-rockchip/efuse.c | 165 + arch/arm/mach-rockchip/efuse.h | 15 ++ is it possible that you forgot Kconfig and Makefile? Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] rockchip: efuse: add efuse driver for rk3288 efuse
Hi Jianqun, Jianqun Xu jay...@rock-chips.com hat am 1. Dezember 2014 um 08:34 geschrieben: Add driver for efuse found on rk3288 board based on rk3288 SoC. Driver will read fuse information of chip at the boot stage of kernel, this information new is for further usage. Signed-off-by: Jianqun Xu jay...@rock-chips.com --- arch/arm/mach-rockchip/efuse.c | 165 + arch/arm/mach-rockchip/efuse.h | 15 2 files changed, 180 insertions(+) create mode 100644 arch/arm/mach-rockchip/efuse.c create mode 100644 arch/arm/mach-rockchip/efuse.h diff --git a/arch/arm/mach-rockchip/efuse.c b/arch/arm/mach-rockchip/efuse.c new file mode 100644 index 000..326d81e --- /dev/null +++ b/arch/arm/mach-rockchip/efuse.c @@ -0,0 +1,165 @@ +/* mach-rockchip/efuse.c + * + * Copyright (c) 2014 Rockchip Electronics Co. Ltd. + * Author: Jianqun Xu jay...@rock-chips.com + * + * Tmis program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * Tmis program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * tmis program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110, USA as far as i know this address is outdated and should be removed. + * + * Tme full GNU General Public License is included in this distribution in the + * file called LICENSE. + */ + +#include linux/kernel.h +#include linux/module.h +#include linux/device.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/of_address.h +#include linux/io.h Please order the includes alphabetical. + +#include efuse.h + +#define EFUSE_BUF_SIZE (32) +#define EFUSE_BUF_LKG_CPU (23) +#define EFUSE_BUF_LKG_GPU (24) +#define EFUSE_BUF_LKG_LOG (25) + +struct rk_efuse_info { + /* Platform device */ + struct device *dev; I think it's not really necessary to store this in the driver data. Better call the relevant functions with struct platform_device as parameter and get your driver data from their. + + /* Hardware resources */ + void __iomem *regs; + + /* buffer to store registers' values */ + unsigned int buf[EFUSE_BUF_SIZE]; The variable name buf isn't very helpful. +}; + +static void efuse_writel(struct rk_efuse_info *efuse, + unsigned int value, + unsigned int offset) +{ + writel_relaxed(value, efuse-regs + offset); +} + +static unsigned int efuse_readl(struct rk_efuse_info *efuse, + unsigned int offset) +{ + return readl_relaxed(efuse-regs + offset); +} + +static unsigned int rockchip_efuse_leakage(struct rk_efuse_info *efuse, + int channel) +{ + switch (channel) { + case EFUSE_BUF_LKG_CPU: + case EFUSE_BUF_LKG_GPU: + case EFUSE_BUF_LKG_LOG: + return efuse-buf[channel]; + default: + dev_err(efuse-dev, unknown channel\n); + return -EINVAL; Returning a negative value in a function with unsigned return type isn't good. Printing only unknown channel isn't optimal, it would be more helpful to print also the invalid value. + } + + return 0; Looks like unreachable code, maybe you could move the default case above down. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] clk: mxs: Fix invalid 32-bit access to frac registers
Hi, Marek Vasut ma...@denx.de hat am 28. Dezember 2014 um 19:30 geschrieben: On Sunday, December 28, 2014 at 11:26:42 AM, Stefan Wahren wrote: According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. This patch fixes the erroneous 32-bit access to these registers and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested only with a i.MX28 board, because i don't have access to an i.MX23 board. since i've also a i.MX23 board, i tested this patch successful. Any conserns about it? Signed-off-by: Stefan Wahren stefan.wah...@i2se.com I don't see a problem, Reviewed-by: Marek Vasut ma...@denx.de Best regards, Marek Vasut ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4 V2] dt-bindings: Add root properties for Raspberry Pi
This patch adds root compatible properties for the following boards: - Raspberry Pi Model A - Raspberry Pi Model A+ - Raspberry Pi Model B - Raspberry Pi Model B (no P5) - Raspberry Pi Model B rev2 - Raspberry Pi Model B+ - Raspberry Pi Compute Module Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- Changes in V2: - add all currently known Raspberry Pi boards as suggested by Stephen Warren Documentation/devicetree/bindings/arm/bcm2835.txt | 31 +++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/bcm2835.txt b/Documentation/devicetree/bindings/arm/bcm2835.txt index ac68348..c78576b 100644 --- a/Documentation/devicetree/bindings/arm/bcm2835.txt +++ b/Documentation/devicetree/bindings/arm/bcm2835.txt @@ -1,8 +1,35 @@ Broadcom BCM2835 device tree bindings --- -Boards with the BCM2835 SoC shall have the following properties: +Raspberry Pi Model A +Required root node properties: +compatible = raspberrypi,model-a, brcm,bcm2835; -Required root node property: +Raspberry Pi Model A+ +Required root node properties: +compatible = raspberrypi,model-a-plus, brcm,bcm2835; +Raspberry Pi Model B +Required root node properties: +compatible = raspberrypi,model-b, brcm,bcm2835; + +Raspberry Pi Model B (no P5) +early model B with I2C0 rather than I2C1 routed to the expansion header +Required root node properties: +compatible = raspberrypi,model-b-i2c0, brcm,bcm2835; + +Raspberry Pi Model B rev2 +Required root node properties: +compatible = raspberrypi,model-b-rev2, brcm,bcm2835; + +Raspberry Pi Model B+ +Required root node properties: +compatible = raspberrypi,model-b-plus, brcm,bcm2835; + +Raspberry Pi Compute Module +Required root node properties: +compatible = raspberrypi,compute-module, brcm,bcm2835; + +Generic BCM2835 board +Required root node properties: compatible = brcm,bcm2835; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4 V2] dt-bindings: Add root properties for Raspberry Pi
Hi Stephen, Stephen Warren swar...@wwwdotorg.org hat am 19. Januar 2015 um 18:13 geschrieben: On 01/19/2015 04:00 AM, Stefan Wahren wrote: This patch adds root compatible properties for the following boards: - Raspberry Pi Model A - Raspberry Pi Model A+ - Raspberry Pi Model B - Raspberry Pi Model B (no P5) - Raspberry Pi Model B rev2 - Raspberry Pi Model B+ - Raspberry Pi Compute Module Acked-by: Stephen Warren swar...@wwwdotorg.org This seems sane to me. One thing I should have asked: What compatible values does the RPi Foundation's downstream for all these cases? good point. Unfortunately my patches to linux-rpi-kernel are held back because of too many recipients, so it's possible that someone complain later. I have looked at chapter 3.1 in [1], but didn't find the any other variants than B and B+: Here, the presence or absence of the -plus is the significant thing, not the b -- Model A's and A+'s will use the b and b-plus variants, respectively. Also in the newest branch [2]: bcm2835-rpi-b.dts - model = Raspberry Pi Model B bcm2708-rpi-b.dts - model = Raspberry Pi Model B bcm2708-rpi-b-plus.dts - model = Raspberry Pi Model B+ Sorry, i don't have any experience with the downstream kernel. So please correct me if looked at the wrong places. [1] - https://github.com/raspberrypi/documentation/blob/master/configuration/device-tree.md [2] - https://github.com/raspberrypi/linux/tree/rpi-3.18.y/ Aligning with that would be nice if possible. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] ARM: bcm2835: Add header file for pinctrl constants
This new header file defines pincontrol constants to use from bcm2835 DTS files for pincontrol properties option. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- include/dt-bindings/pinctrl/bcm2835.h | 27 +++ 1 file changed, 27 insertions(+) create mode 100644 include/dt-bindings/pinctrl/bcm2835.h diff --git a/include/dt-bindings/pinctrl/bcm2835.h b/include/dt-bindings/pinctrl/bcm2835.h new file mode 100644 index 000..6f0bc37 --- /dev/null +++ b/include/dt-bindings/pinctrl/bcm2835.h @@ -0,0 +1,27 @@ +/* + * Header providing constants for bcm2835 pinctrl bindings. + * + * Copyright (C) 2015 Stefan Wahren stefan.wah...@i2se.com + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#ifndef __DT_BINDINGS_PINCTRL_BCM2835_H__ +#define __DT_BINDINGS_PINCTRL_BCM2835_H__ + +/* brcm,function property */ +#define BCM2835_FSEL_GPIO_IN 0 +#define BCM2835_FSEL_GPIO_OUT 1 +#define BCM2835_FSEL_ALT5 2 +#define BCM2835_FSEL_ALT4 3 +#define BCM2835_FSEL_ALT0 4 +#define BCM2835_FSEL_ALT1 5 +#define BCM2835_FSEL_ALT2 6 +#define BCM2835_FSEL_ALT3 7 + +#endif /* __DT_BINDINGS_PINCTRL_BCM2835_H__ */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] dt-bindings: Add vendor prefix for Raspberry Pi
Since the prefix is already in use, we need to add it in the vendor list. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../devicetree/bindings/vendor-prefixes.txt|1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 5d2251a..0546f73 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -136,6 +136,7 @@ radxa Radxa raidsonic RaidSonic Technology GmbH ralink Mediatek/Ralink Technology Corp. ramtronRamtron International +raspberrypiRaspberry Pi Foundation realtek Realtek Semiconductor Corp. renesasRenesas Electronics Corporation ricoh Ricoh Co. Ltd. -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] ARM: bcm2835: DT improvements
This patch series contains DT improvements for the Raspberry Pi. Patch 1,2: Add missing vendor prefix and root compatible properties Patch 3,4: Use constants for pin function instead of error-prone numbers Stefan Wahren (4): dt-bindings: Add vendor prefix for Raspberry Pi dt-bindings: Add root properties for Raspberry Pi B and B+ ARM: bcm2835: Add header file for pinctrl constants ARM: bcm2835: Use pinctrl header Documentation/devicetree/bindings/arm/bcm2835.txt | 10 ++-- .../devicetree/bindings/vendor-prefixes.txt|1 + arch/arm/boot/dts/bcm2835-rpi-b-plus.dts |4 +-- arch/arm/boot/dts/bcm2835-rpi-b.dts|4 +-- arch/arm/boot/dts/bcm2835-rpi.dtsi |8 +++--- arch/arm/boot/dts/bcm2835.dtsi |3 ++- include/dt-bindings/pinctrl/bcm2835.h | 27 7 files changed, 46 insertions(+), 11 deletions(-) create mode 100644 include/dt-bindings/pinctrl/bcm2835.h -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] dt-bindings: Add root properties for Raspberry Pi B and B+
This patch adds root compatible properties for the following boards: - Raspberry Pi Model B - Raspberry Pi Model B+ Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- Documentation/devicetree/bindings/arm/bcm2835.txt | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/bcm2835.txt b/Documentation/devicetree/bindings/arm/bcm2835.txt index ac68348..789aca2 100644 --- a/Documentation/devicetree/bindings/arm/bcm2835.txt +++ b/Documentation/devicetree/bindings/arm/bcm2835.txt @@ -1,8 +1,14 @@ Broadcom BCM2835 device tree bindings --- -Boards with the BCM2835 SoC shall have the following properties: +Raspberry Pi Model B +Required root node properties: +compatible = raspberrypi,model-b, brcm,bcm2835; -Required root node property: +Raspberry Pi Model B+ +Required root node properties: +compatible = raspberrypi,model-b-plus, brcm,bcm2835; +Generic BCM2835 board +Required root node properties: compatible = brcm,bcm2835; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] ARM: bcm2835: Use pinctrl header
This patch converts all bcm2835 dts files to use the pinctrl header file. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- arch/arm/boot/dts/bcm2835-rpi-b-plus.dts |4 ++-- arch/arm/boot/dts/bcm2835-rpi-b.dts |4 ++-- arch/arm/boot/dts/bcm2835-rpi.dtsi |8 arch/arm/boot/dts/bcm2835.dtsi |3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/arch/arm/boot/dts/bcm2835-rpi-b-plus.dts b/arch/arm/boot/dts/bcm2835-rpi-b-plus.dts index e479515..668442b 100644 --- a/arch/arm/boot/dts/bcm2835-rpi-b-plus.dts +++ b/arch/arm/boot/dts/bcm2835-rpi-b-plus.dts @@ -1,5 +1,5 @@ /dts-v1/; -/include/ bcm2835-rpi.dtsi +#include bcm2835-rpi.dtsi / { compatible = raspberrypi,model-b-plus, brcm,bcm2835; @@ -25,6 +25,6 @@ /* I2S interface */ i2s_alt0: i2s_alt0 { brcm,pins = 18 19 20 21; - brcm,function = 4; /* alt0 */ + brcm,function = BCM2835_FSEL_ALT0; }; }; diff --git a/arch/arm/boot/dts/bcm2835-rpi-b.dts b/arch/arm/boot/dts/bcm2835-rpi-b.dts index bafa46f..ee89b79 100644 --- a/arch/arm/boot/dts/bcm2835-rpi-b.dts +++ b/arch/arm/boot/dts/bcm2835-rpi-b.dts @@ -1,5 +1,5 @@ /dts-v1/; -/include/ bcm2835-rpi.dtsi +#include bcm2835-rpi.dtsi / { compatible = raspberrypi,model-b, brcm,bcm2835; @@ -18,6 +18,6 @@ /* I2S interface */ i2s_alt2: i2s_alt2 { brcm,pins = 28 29 30 31; - brcm,function = 6; /* alt2 */ + brcm,function = BCM2835_FSEL_ALT2; }; }; diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi index c706448..46780bb 100644 --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi @@ -1,4 +1,4 @@ -/include/ bcm2835.dtsi +#include bcm2835.dtsi / { memory { @@ -21,17 +21,17 @@ gpioout: gpioout { brcm,pins = 6; - brcm,function = 1; /* GPIO out */ + brcm,function = BCM2835_FSEL_GPIO_OUT; }; alt0: alt0 { brcm,pins = 0 1 2 3 4 5 7 8 9 10 11 14 15 40 45; - brcm,function = 4; /* alt0 */ + brcm,function = BCM2835_FSEL_ALT0; }; alt3: alt3 { brcm,pins = 48 49 50 51 52 53; - brcm,function = 7; /* alt3 */ + brcm,function = BCM2835_FSEL_ALT3; }; }; diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi index 3342cb1..be9c914 100644 --- a/arch/arm/boot/dts/bcm2835.dtsi +++ b/arch/arm/boot/dts/bcm2835.dtsi @@ -1,4 +1,5 @@ -/include/ skeleton.dtsi +#include dt-bindings/pinctrl/bcm2835.h +#include skeleton.dtsi / { compatible = brcm,bcm2835; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Fabio Estevam feste...@gmail.com hat am 11. Februar 2015 um 17:58 geschrieben: Hi Stefan, On Tue, Feb 10, 2015 at 7:24 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 10. Februar 2015 um 16:06 geschrieben: Hi Stefan, On Tue, Feb 10, 2015 at 11:55 AM, Stefan Wahren stefan.wah...@i2se.com wrote: Could you apply only the clk-imx28.c part of my patch and see what happens? If I apply only the clk-imx28.c part of your patch I can successfully probe the SPI NOR. thanks this is very helpful. I built the linux-next for my Duckbill and add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts. Without my patch i get the following for HW_CLKCTRL_FRAC0: ./memwatch -a 0x800401B0 0x800401b0: 0x5e5b5513 With my patch i get: ./memwatch -a 0x800401B0 0x800401b0: 0x5e1b5513 I always get 0x5e5b5513 with or without your patch. very strange. Do you have any idea why IO1_STABLE is permanent low? Here are the register dumps as requested by Mike. Without this patch after boot 0x8004: 0x0006 0x80040010: 0x84b1 0x80040050: 0x00011001 0x80040080: 0x6401 0x80040090: 0x0001 0x800400a0: 0x8001 0x800400b0: 0x0002 0x800400c0: 0xa001 0x800401b0: 0x5e5b5513 0x800401c0: 0x00929292 0x800401d0: 0x4104 With this patch after boot 0x8004: 0x0006 0x80040010: 0x84b1 0x80040050: 0x00011001 0x80040080: 0x6401 0x80040090: 0x0001 0x800400a0: 0x8001 0x800400b0: 0x0002 0x800400c0: 0xa001 0x800401b0: 0x5e1b5513 0x800401c0: 0x00929292 0x800401d0: 0x4104 So only IO1_STABLE (0x800401b0) is different. If i restore clk_ref_set_rate() from clk-ref.c after the complete patch is applied, the issue with the IO1_STABLE bit disappears. Can you confirm the behavior according to your flash issue? I think it would be the best to revert my patch. Sorry for the trouble. Stefan ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Fabio Estevam feste...@gmail.com hat am 12. Februar 2015 um 20:08 geschrieben: Hi Stefan, On Thu, Feb 12, 2015 at 4:59 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 11. Februar 2015 um 22:10 geschrieben: On Wed, Feb 11, 2015 at 6:31 PM, Stefan Wahren stefan.wah...@i2se.com wrote: I always get 0x5e5b5513 with or without your patch. very strange. Do you have any idea why IO1_STABLE is permanent low? On my case it is always 1. i expected the same behavior on my hardware. Do you use u-boot as bootloader? Yes, I do. i will try to test it with u-boot. Can you confirm the behavior according to your flash issue? In my tests IO1_STABLE stays the same. This wasn't the intension of my second question. I wanted to know about the state of the SPI NOR flash detection process. Does it sucessed if you apply the patch, but revert the changes in clk_ref_set_rate() from clk-ref.c? I don't have my mx28evk setup available at the moment, but when I applied your patch and reverted all the changes in clk-ref.c, then the SPI flash detection works. I haven't tested to only reverting the changes inside clk_ref_set_rate(), but I can do it tomorrow. I think the reason for the problem in the flash detection is caused by the misaligned access on the frac register. Maybe you want to try the following after the second patch is reverted. Stefan 8-- diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..87969e3 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -73,8 +73,10 @@ static void __init clk_misc_init(void) * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + val = readl_relaxed(FRAC); + val = ~(0x3f BP_FRAC_IOFRAC); + val |= 30 BP_FRAC_IOFRAC; + writel_relaxed(val, FRAC); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..e47ad69 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -118,10 +118,15 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must modify frac bytewise. */ val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); + val = ~(0x3f BP_FRAC0_IO0FRAC); + val |= 30 BP_FRAC0_IO0FRAC; + writel_relaxed(val, FRAC0); + val = readl_relaxed(FRAC0); + val = ~(0x3f BP_FRAC0_IO1FRAC); + val |= 30 BP_FRAC0_IO1FRAC; writel_relaxed(val, FRAC0); } -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Fabio Estevam feste...@gmail.com hat am 11. Februar 2015 um 22:10 geschrieben: On Wed, Feb 11, 2015 at 6:31 PM, Stefan Wahren stefan.wah...@i2se.com wrote: I always get 0x5e5b5513 with or without your patch. very strange. Do you have any idea why IO1_STABLE is permanent low? On my case it is always 1. i expected the same behavior on my hardware. Do you use u-boot as bootloader? Can you confirm the behavior according to your flash issue? In my tests IO1_STABLE stays the same. This wasn't the intension of my second question. I wanted to know about the state of the SPI NOR flash detection process. Does it sucessed if you apply the patch, but revert the changes in clk_ref_set_rate() from clk-ref.c? I think it would be the best to revert my patch. Sorry for the trouble. To be on the safe side, I agree. Could you please send a revert patch? Sure. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Revert clk: mxs: Fix invalid 32-bit access to frac registers
Revert commit 039e59707507 (clk: mxs: Fix invalid 32-bit access to frac registers), because it leads to a faulty spi communication on mx28evk. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com Reported-by: Fabio Estevam fabio.este...@freescale.com --- drivers/clk/mxs/clk-imx23.c | 11 +++ drivers/clk/mxs/clk-imx28.c | 19 ++- drivers/clk/mxs/clk-ref.c | 19 +-- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index a084566..9fc9359 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,13 +46,11 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 - -#define FRAC_IO3 +#define BP_FRAC_IOFRAC 24 static void __init clk_misc_init(void) { u32 val; - u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -74,12 +72,9 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. -* According to reference manual we must access frac bytewise. */ - frac = readb_relaxed(FRAC + FRAC_IO); - frac = ~0x3f; - frac |= 30; - writeb_relaxed(frac, FRAC + FRAC_IO); + writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); + writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index c541377..a6c3501 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,9 +53,8 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 - -#define FRAC0_IO1 2 -#define FRAC0_IO0 3 +#define BP_FRAC0_IO1FRAC 16 +#define BP_FRAC0_IO0FRAC 24 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -86,7 +85,6 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; - u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -120,16 +118,11 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. -* According to reference manual we must access frac0 bytewise. */ - frac = readb_relaxed(FRAC0 + FRAC0_IO0); - frac = ~0x3f; - frac |= 30; - writeb_relaxed(frac, FRAC0 + FRAC0_IO0); - frac = readb_relaxed(FRAC0 + FRAC0_IO1); - frac = ~0x3f; - frac |= 30; - writeb_relaxed(frac, FRAC0 + FRAC0_IO1); + val = readl_relaxed(FRAC0); + val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); + val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); + writel_relaxed(val, FRAC0); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index ad3851c..4adeed6 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,8 +16,6 @@ #include linux/slab.h #include clk.h -#define BF_CLKGATE BIT(7) - /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -41,7 +39,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + CLR); + writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); return 0; } @@ -50,7 +48,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + SET); + writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -58,7 +56,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate; - u8 frac = readb_relaxed(ref-reg + ref-idx) 0x3f; + u8 frac = (readl_relaxed(ref-reg) (ref-idx * 8)) 0x3f; tmp *= 18; do_div(tmp, frac); @@ -95,7 +93,8 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, struct clk_ref *ref = to_clk_ref(hw); unsigned long flags; u64 tmp = parent_rate; - u8 frac, val; + u32 val; + u8 frac, shift = ref-idx * 8; tmp = tmp * 18 + rate / 2; do_div(tmp, rate); @@ -108,10 +107,10 @@ static int clk_ref_set_rate(struct clk_hw *hw, unsigned long rate, spin_lock_irqsave(mxs_lock, flags); - val = readb_relaxed(ref-reg
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Stefan Wahren stefan.wah...@i2se.com hat am 11. Februar 2015 um 21:31 geschrieben: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 11. Februar 2015 um 17:58 geschrieben: Hi Stefan, On Tue, Feb 10, 2015 at 7:24 PM, Stefan Wahren stefan.wah...@i2se.com wrote: Hi Fabio, Fabio Estevam feste...@gmail.com hat am 10. Februar 2015 um 16:06 geschrieben: Hi Stefan, On Tue, Feb 10, 2015 at 11:55 AM, Stefan Wahren stefan.wah...@i2se.com wrote: Could you apply only the clk-imx28.c part of my patch and see what happens? If I apply only the clk-imx28.c part of your patch I can successfully probe the SPI NOR. thanks this is very helpful. I built the linux-next for my Duckbill and add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts. Without my patch i get the following for HW_CLKCTRL_FRAC0: ./memwatch -a 0x800401B0 0x800401b0: 0x5e5b5513 With my patch i get: ./memwatch -a 0x800401B0 0x800401b0: 0x5e1b5513 I always get 0x5e5b5513 with or without your patch. very strange. Do you have any idea why IO1_STABLE is permanent low? Here are the register dumps as requested by Mike. Without this patch after boot 0x8004: 0x0006 0x80040010: 0x84b1 0x80040050: 0x00011001 0x80040080: 0x6401 0x80040090: 0x0001 0x800400a0: 0x8001 0x800400b0: 0x0002 0x800400c0: 0xa001 0x800401b0: 0x5e5b5513 0x800401c0: 0x00929292 0x800401d0: 0x4104 With this patch after boot 0x8004: 0x0006 0x80040010: 0x84b1 0x80040050: 0x00011001 0x80040080: 0x6401 0x80040090: 0x0001 0x800400a0: 0x8001 0x800400b0: 0x0002 0x800400c0: 0xa001 0x800401b0: 0x5e1b5513 0x800401c0: 0x00929292 0x800401d0: 0x4104 just for the records here is the register dump for the case patch V2 on Duckbill with U-Boot instead of imx-bootlets: 0x8004: 0x0006 0x80040010: 0x84b1 0x80040050: 0x00011001 0x80040080: 0x6401 0x80040090: 0x0005 0x800400a0: 0x8001 0x800400b0: 0x0002 0x800400c0: 0xa001 0x800401b0: 0x5e5b5513 0x800401c0: 0x00929292 0x800401d0: 0x4104 So the issue with IO1_STABLE seems to come from the bootloader. Do you have any idea which i.MX28 registers additionally could be relevant? Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] clk: mxs: Fix invalid 32-bit access to frac registers
Hi, Am 28.01.2015 um 04:36 schrieb Zhi Li: On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette mturque...@linaro.org wrote: Quoting Marek Vasut (2015-01-21 15:39:01) On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren stefan.wah...@i2se.com wrote: According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. I don't think mx23 and mx28 have such limitation. I will double check with IC team about this. RTL is generated from a xml file. All registers implement is unified. I don't think only clock control register have such limitation and other registers not. Hi, Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) states otherwise, but maybe the documentation is simply not matching the silicon. Here's a quote: This register controls the 9-phase fractional clock dividers. The fractional clock frequencies are a product of the values in these registers. NOTE: This register can only be addressed by byte instructions. Addressing word or half- word are not allowed. I also recall seeing weird behavior when these registers were accessed by word access in U-Boot, so I believe the datasheet is correct. Hi Frank, Are you satisfied with this patch? I asked IC designer about this. They will check RTL code. I will check their status again. Our released BSP code used 32bit WORD access. i want to point out that the 32bit WORD is divided in 4 parts (IO0FRAC, IO1FRAC, EMIFRAC, CPUFRAC). Yes, it's true that BSP code access the register as 32bit, but it's never modify the complete 32bit at once just only 1 part (8bit) at a time. So here is my theory about Fractional Clock Control Register: Reading as 32bit WORD = safe Modify only 1 part (8bit) of the 32bit WORD = safe Modify more than 1 part of the 32bit WORD = unsafe !!! Best regards Stefan best regards Frank Li Regards, Mike Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] clk: mxs: Fix invalid 32-bit access to frac registers
Hi, Am 28.01.2015 um 17:10 schrieb Zhi Li: On Wed, Jan 28, 2015 at 9:52 AM, Stefan Wahren stefan.wah...@i2se.com wrote: Hi, Am 28.01.2015 um 04:36 schrieb Zhi Li: On Tue, Jan 27, 2015 at 7:51 PM, Mike Turquette mturque...@linaro.org wrote: Quoting Marek Vasut (2015-01-21 15:39:01) On Wednesday, January 21, 2015 at 05:16:03 PM, Zhi Li wrote: On Sun, Dec 28, 2014 at 4:26 AM, Stefan Wahren stefan.wah...@i2se.com wrote: According to i.MX23 and i.MX28 reference manual the fractional clock control registers must be addressed by byte instructions. I don't think mx23 and mx28 have such limitation. I will double check with IC team about this. RTL is generated from a xml file. All registers implement is unified. I don't think only clock control register have such limitation and other registers not. Hi, Section 10.8.24 in the MX28 datasheet (Fractional Clock Control Register 0) states otherwise, but maybe the documentation is simply not matching the silicon. Here's a quote: This register controls the 9-phase fractional clock dividers. The fractional clock frequencies are a product of the values in these registers. NOTE: This register can only be addressed by byte instructions. Addressing word or half- word are not allowed. I also recall seeing weird behavior when these registers were accessed by word access in U-Boot, so I believe the datasheet is correct. Hi Frank, Are you satisfied with this patch? I asked IC designer about this. They will check RTL code. I will check their status again. Our released BSP code used 32bit WORD access. i want to point out that the 32bit WORD is divided in 4 parts (IO0FRAC, IO1FRAC, EMIFRAC, CPUFRAC). Yes, it's true that BSP code access the register as 32bit, but it's never modify the complete 32bit at once just only 1 part (8bit) at a time. So here is my theory about Fractional Clock Control Register: Reading as 32bit WORD = safe Modify only 1 part (8bit) of the 32bit WORD = safe Modify more than 1 part of the 32bit WORD = unsafe !!! Yes, it is align with what I get from IC designer. best regards Frank Li okay, fine. The clk init for the i.MX28 in the kernel tries to the set IO0FRAC and IO1FRAC at once. So this patch fix this problem. And since i was on that, i changed all accesses on that register to 8bit to avoid this problem in the future. So what's your opinion about this patch? Best regards Stefan Best regards Stefan best regards Frank Li Regards, Mike Best regards, Marek Vasut ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
According to i.MX23 and i.MX28 reference manual [1],[2] the fractional clock control register is 32-bit wide, but is separated in 4 parts. So write instructions must not apply to more than 1 part at once. The clk init for the i.MX28 violates this restriction and all the other accesses on that register suggest that there isn't such a restriction. This patch restricts the access to this register to byte instructions and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested with a i.MX23 and a i.MX28 board. [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf [2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf Signed-off-by: Stefan Wahren stefan.wah...@i2se.com Reviewed-by: Marek Vasut ma...@denx.de --- Changes in V2: - use relaxed access operations in clk-ref drivers/clk/mxs/clk-imx23.c | 11 --- drivers/clk/mxs/clk-imx28.c | 19 +-- drivers/clk/mxs/clk-ref.c | 19 ++- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..a084566 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,11 +46,13 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO3 static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -72,9 +74,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. +* According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + frac = readb_relaxed(FRAC + FRAC_IO); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC + FRAC_IO); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..c541377 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -118,11 +120,16 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + frac = readb_relaxed(FRAC0 + FRAC0_IO0); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); + frac = readb_relaxed(FRAC0 + FRAC0_IO1); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..ad3851c 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate
[PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
According to i.MX23 and i.MX28 reference manual [1],[2] the fractional clock control register is 32-bit wide, but is separated in 4 parts. So write instructions must not apply to more than 1 part at once. The clk init for the i.MX28 violates this restriction and all the other accesses on that register suggest that there isn't such a restriction. This patch restricts the access to this register to byte instructions and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested with a i.MX23 and a i.MX28 board. [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf [2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf Signed-off-by: Stefan Wahren stefan.wah...@i2se.com Reviewed-by: Marek Vasut ma...@denx.de --- Changes in V2: - use relaxed access operations in clk-ref drivers/clk/mxs/clk-imx23.c | 11 --- drivers/clk/mxs/clk-imx28.c | 19 +-- drivers/clk/mxs/clk-ref.c | 19 ++- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c index 9fc9359..a084566 100644 --- a/drivers/clk/mxs/clk-imx23.c +++ b/drivers/clk/mxs/clk-imx23.c @@ -46,11 +46,13 @@ static void __iomem *digctrl; #define BP_CLKSEQ_BYPASS_SAIF 0 #define BP_CLKSEQ_BYPASS_SSP 5 #define BP_SAIF_DIV_FRAC_EN16 -#define BP_FRAC_IOFRAC 24 + +#define FRAC_IO3 static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -72,9 +74,12 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac to get a 288 MHz ref_io. +* According to reference manual we must access frac bytewise. */ - writel_relaxed(0x3f BP_FRAC_IOFRAC, FRAC + CLR); - writel_relaxed(30 BP_FRAC_IOFRAC, FRAC + SET); + frac = readb_relaxed(FRAC + FRAC_IO); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC + FRAC_IO); } static const char *sel_pll[] __initconst = { pll, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..c541377 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -53,8 +53,9 @@ static void __iomem *clkctrl; #define BP_ENET_SLEEP 31 #define BP_CLKSEQ_BYPASS_SAIF0 0 #define BP_CLKSEQ_BYPASS_SSP0 3 -#define BP_FRAC0_IO1FRAC 16 -#define BP_FRAC0_IO0FRAC 24 + +#define FRAC0_IO1 2 +#define FRAC0_IO0 3 static void __iomem *digctrl; #define DIGCTRL digctrl @@ -85,6 +86,7 @@ int mxs_saif_clkmux_select(unsigned int clkmux) static void __init clk_misc_init(void) { u32 val; + u8 frac; /* Gate off cpu clock in WFI for power saving */ writel_relaxed(1 BP_CPU_INTERRUPT_WAIT, CPU + SET); @@ -118,11 +120,16 @@ static void __init clk_misc_init(void) /* * 480 MHz seems too high to be ssp clock source directly, * so set frac0 to get a 288 MHz ref_io0 and ref_io1. +* According to reference manual we must access frac0 bytewise. */ - val = readl_relaxed(FRAC0); - val = ~((0x3f BP_FRAC0_IO0FRAC) | (0x3f BP_FRAC0_IO1FRAC)); - val |= (30 BP_FRAC0_IO0FRAC) | (30 BP_FRAC0_IO1FRAC); - writel_relaxed(val, FRAC0); + frac = readb_relaxed(FRAC0 + FRAC0_IO0); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO0); + frac = readb_relaxed(FRAC0 + FRAC0_IO1); + frac = ~0x3f; + frac |= 30; + writeb_relaxed(frac, FRAC0 + FRAC0_IO1); } static const char *sel_cpu[] __initconst = { ref_cpu, ref_xtal, }; diff --git a/drivers/clk/mxs/clk-ref.c b/drivers/clk/mxs/clk-ref.c index 4adeed6..ad3851c 100644 --- a/drivers/clk/mxs/clk-ref.c +++ b/drivers/clk/mxs/clk-ref.c @@ -16,6 +16,8 @@ #include linux/slab.h #include clk.h +#define BF_CLKGATE BIT(7) + /** * struct clk_ref - mxs reference clock * @hw: clk_hw for the reference clock @@ -39,7 +41,7 @@ static int clk_ref_enable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + CLR); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + CLR); return 0; } @@ -48,7 +50,7 @@ static void clk_ref_disable(struct clk_hw *hw) { struct clk_ref *ref = to_clk_ref(hw); - writel_relaxed(1 ((ref-idx + 1) * 8 - 1), ref-reg + SET); + writeb_relaxed(BF_CLKGATE, ref-reg + ref-idx + SET); } static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, @@ -56,7 +58,7 @@ static unsigned long clk_ref_recalc_rate(struct clk_hw *hw, { struct clk_ref *ref = to_clk_ref(hw); u64 tmp = parent_rate
Re: [PATCH v3 04/20] power_supply: sysfs: Use power_supply_*() API for accessing function attrs
Hi Krzysztof, Krzysztof Kozlowski k.kozlow...@samsung.com hat am 30. Januar 2015 um 15:47 geschrieben: Replace direct calls to power supply function attributes with wrappers. Wrappers provide safe access in case of unregistering the power supply (e.g. by removing the driver). Replace: - get_property - power_supply_get_property - set_property - power_supply_set_property - property_is_writeable - power_supply_property_is_writeable Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com Acked-by: Jonghwa Lee jonghwa3@samusng.com just a nit. It looks like a typo in the mail address which is also in patch 5 - 10. I applied patch 1 - 14 to my repo with a upcoming power driver (mxs_power) and didn't see any problems with your patches. Stefan Acked-by: Pavel Machek pa...@ucw.cz Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Reviewed-by: Sebastian Reichel s...@kernel.org --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Am 10.02.2015 um 14:09 schrieb Fabio Estevam: Hi Stefan, On Tue, Feb 10, 2015 at 11:05 AM, Stefan Wahren stefan.wah...@i2se.com wrote: sorry no. But i will try to get a mx28evk to reproduce the problem and Just a comment: mx28evk comes without SPI NOR flash from the factory. I have populated one in my mx28evk (slot U49). i tought it would be easy because of this nice flash socket, but it's for NAND :-( Could you apply only the clk-imx28.c part of my patch and see what happens? Stefan narrow down which part of the patch causes this problem. Did you see any problem with the clock control register settings? I haven't inspected the clock control registers yet. Regards, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Fabio Estevam feste...@gmail.com hat am 10. Februar 2015 um 16:06 geschrieben: Hi Stefan, On Tue, Feb 10, 2015 at 11:55 AM, Stefan Wahren stefan.wah...@i2se.com wrote: Could you apply only the clk-imx28.c part of my patch and see what happens? If I apply only the clk-imx28.c part of your patch I can successfully probe the SPI NOR. thanks this is very helpful. I built the linux-next for my Duckbill and add the SSP2 section from imx28-evk.dts into imx28-duckbill.dts. Without my patch i get the following for HW_CLKCTRL_FRAC0: ./memwatch -a 0x800401B0 0x800401b0: 0x5e5b5513 With my patch i get: ./memwatch -a 0x800401B0 0x800401b0: 0x5e1b5513 So it looks like a problem in my patch. I orded such a flash chip, but it will take some time until i can use it with my hardware. Stefan Thanks ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 RESEND] clk: mxs: Fix invalid 32-bit access to frac registers
Hi Fabio, Am 10.02.2015 um 13:52 schrieb Fabio Estevam: Hi Stefan, On Fri, Jan 30, 2015 at 5:20 PM, Stefan Wahren stefan.wah...@i2se.com wrote: According to i.MX23 and i.MX28 reference manual [1],[2] the fractional clock control register is 32-bit wide, but is separated in 4 parts. So write instructions must not apply to more than 1 part at once. The clk init for the i.MX28 violates this restriction and all the other accesses on that register suggest that there isn't such a restriction. This patch restricts the access to this register to byte instructions and extends the comment in the init functions. Btw the imx23 init now uses a R-M-W sequence just like imx28 init to avoid any clock glitches. The changes has been tested with a i.MX23 and a i.MX28 board. [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf [2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf Signed-off-by: Stefan Wahren stefan.wah...@i2se.com Reviewed-by: Marek Vasut ma...@denx.de --- Changes in V2: - use relaxed access operations in clk-ref With this patch applied mx28evk cannot probe SPI NOR flash: m25p80 spi1.0: unrecognized JEDEC id bytes: bf, 24, 40 Reverting it from linux-next, then the SPI NOR probe correctly. m25p80 spi1.0: sst25vf016b (2048 Kbytes) Any ideas? sorry no. But i will try to get a mx28evk to reproduce the problem and narrow down which part of the patch causes this problem. Did you see any problem with the clock control register settings? Stefan Thanks for the suggestion, Marek! Regards, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: parent/child hierarchy for regulator
Hi Peter, Am 05.03.2015 um 11:35 schrieb Peter Chen: Hi lists, Any good ways at code/dts to show parent/child hierarchy for regulator? The related regulators at my platforms like below: PMIC (SWB 5v) -- Switch Chip (GPIO Regulator) -- USB VBUS PMIC has one 5V regulator (eg, swbst at pfuse), and it is the input for USB power switch chip, and there are two gpios at this switch chip to control if 5V is output or not, we register these two gpios as fixed regulators, currently, if regulator swbst is disabled, the gpio regulator has no way to know, and cause the vbus voltage is wrong. Thanks in advance. i don't have an answer to your question, but do you think of i.MX28 platform? Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/7] drivers: Introduce new eFuse subsystem stub
Hi, [adding devicetree list] Am 25.02.2015 um 12:45 schrieb Ezequiel Garcia: This commit introduces a new eFuse subsystem stub to hold all the eFuse-like device drivers. This will be used to host the currently supported Tegra eFuse driver, and will allow to add support for other platforms as well. as i mentioned in the old discussion it would be nice to keep drivers and devicetree binding documentation consistent. So how about renaming? Documentation/devicetree/bindings/fuse/ - Documentation/devicetree/bindings/efuse/ Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 3/3] clk: imx28: add DC-DC clock domain
This patch adds the DC-DC clock domain into the i.MX28 clock driver. That enables consumers to change DC-DC clock frequency in order to avoid interferences without changing hardware. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/clk/mxs/clk-imx28.c | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c index a6c3501..de2c347 100644 --- a/drivers/clk/mxs/clk-imx28.c +++ b/drivers/clk/mxs/clk-imx28.c @@ -60,6 +60,18 @@ static void __iomem *digctrl; #define DIGCTRL digctrl #define BP_SAIF_CLKMUX 10 +static void __iomem *powerctrl; +#define POWERCTRL powerctrl +#define POWER_MISC (POWERCTRL + 0x0090) + +static const struct clk_div_table dcdc_div_table[] = { + { .val = 0, .div = 16 }, + { .val = 1, .div = 24 }, + { .val = 2, .div = 20 }, + { .val = 3, .div = 25 }, + { /* sentinel */ }, +}; + /* * HW_SAIF_CLKMUX_SEL: * DIRECT(0x0): SAIF0 clock pins selected for SAIF0 input clocks, and SAIF1 @@ -134,6 +146,8 @@ static const char *sel_pll0[] __initconst = { pll0, ref_xtal, }; static const char *cpu_sels[] __initconst = { cpu_pll, cpu_xtal, }; static const char *emi_sels[] __initconst = { emi_pll, emi_xtal, }; static const char *ptp_sels[] __initconst = { ref_xtal, pll0, }; +static const char *dcdc_plls[] __initconst = { pll0, ref_gpmi, }; +static const char *dcdc_sels[] __initconst = { ref_xtal, dcdc_div, }; enum imx28_clk { ref_xtal, pll0, pll1, pll2, ref_cpu, ref_emi, ref_io0, ref_io1, @@ -145,7 +159,8 @@ enum imx28_clk { clk32k_div, rtc, lradc, spdif_div, clk32k, pwm, uart, ssp0, ssp1, ssp2, ssp3, gpmi, spdif, emi, saif0, saif1, lcdif, etm, fec, can0, can1, usb0, usb1, usb0_phy, usb1_phy, enet_out, - clk_max + dcdc_pll, dcdc_div, dcdc_sel, + clk_max, }; static struct clk *clks[clk_max]; @@ -157,13 +172,18 @@ static enum imx28_clk clks_init_on[] __initdata = { static void __init mx28_clocks_init(struct device_node *np) { - struct device_node *dcnp; + struct device_node *tnp; u32 i; - dcnp = of_find_compatible_node(NULL, NULL, fsl,imx28-digctl); - digctrl = of_iomap(dcnp, 0); + tnp = of_find_compatible_node(NULL, NULL, fsl,imx28-digctl); + digctrl = of_iomap(tnp, 0); WARN_ON(!digctrl); - of_node_put(dcnp); + of_node_put(tnp); + + tnp = of_find_compatible_node(NULL, NULL, fsl,imx28-power); + powerctrl = of_iomap(tnp, 0); + WARN_ON(!powerctrl); + of_node_put(tnp); clkctrl = of_iomap(np, 0); WARN_ON(!clkctrl); @@ -235,6 +255,9 @@ static void __init mx28_clocks_init(struct device_node *np) clks[usb0_phy] = clk_register_gate(NULL, usb0_phy, pll0, 0, PLL0CTRL0, 18, 0, mxs_lock); clks[usb1_phy] = clk_register_gate(NULL, usb1_phy, pll1, 0, PLL1CTRL0, 18, 0, mxs_lock); clks[enet_out] = clk_register_gate(NULL, enet_out, pll2, 0, ENET, 18, 0, mxs_lock); + clks[dcdc_pll] = mxs_clk_mux(dcdc_pll, POWER_MISC, 6, 1, dcdc_plls, ARRAY_SIZE(dcdc_plls)); + clks[dcdc_div] = clk_register_divider_table(NULL, dcdc_div, dcdc_pll, 0, POWER_MISC, 4, 2, 0, dcdc_div_table, mxs_lock); + clks[dcdc_sel] = mxs_clk_mux(dcdc_sel, POWER_MISC, 0, 1, dcdc_sels, ARRAY_SIZE(dcdc_sels)); for (i = 0; i ARRAY_SIZE(clks); i++) if (IS_ERR(clks[i])) { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 2/3] ARM: imx28: define PMU as clock consumer
This patch defines the i.MX28 PMU as consumer of clock dcdc_sel. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- arch/arm/boot/dts/imx28.dtsi |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi index 02330f4..bdd087d 100644 --- a/arch/arm/boot/dts/imx28.dtsi +++ b/arch/arm/boot/dts/imx28.dtsi @@ -1022,7 +1022,9 @@ }; power: power@80044000 { + compatible = fsl,imx28-power, syscon; reg = 0x80044000 0x2000; + clocks = clks 67; status = disabled; }; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 0/3] clk: imx28: add DC-DC clock domain
This patch series integrates the DC-DC clock domain into the i.MX28 clock driver. That enables consumers to change DC-DC clock frequency in order to avoid interferences without changing hardware. The final patch in the near future should handle the i.MX23, too. * patch 1 contains the update for the DT binding documentation * patch 2 defines the PMU as the clock consumer * patch 3 is extension of the clock driver All information about the DC-DC clock was taken from the reference manual [1]. Unfortunately the logical diagram doesn't contain the DC-DC clock domain, so i decide to create a ASCII diagramm (please look at patch 1). The series is related to the upcoming MXS PMU and regulator driver (last patch [2], working repo [3]). At the end i still have a question: The MXS PMU driver will using the syscon interface for accessing the HW_POWER registers. The ctrl register for the DC-DC clock domain is located under HW_POWER and not under HW_CLKCTRL. So should the clock registration be done in clock driver like in patch 3 or in the upcoming PMU driver? Any other comments about the implementation are also welcome. Regards Stefan [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf i.MX28 Applications Processor Reference Manual, Rev. 2, 08/2013 11.12.10 DC-DC Miscellaneous Register (HW_POWER_MISC) [2] - http://marc.info/?l=linux-pmm=142698428606221w=2 [3] - https://github.com/lategoodbye/linux-mxs-power/tree/syscon Stefan Wahren (3): DT: imx28-clock: add ids for DC-DC clock domain ARM: imx28: define PMU as clock consumer clk: imx28: add DC-DC clock domain .../devicetree/bindings/clock/imx28-clock.txt |3 ++ arch/arm/boot/dts/imx28.dtsi |2 ++ drivers/clk/mxs/clk-imx28.c| 33 +--- 3 files changed, 33 insertions(+), 5 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 1/3] DT: imx28-clock: add ids for DC-DC clock domain
This patch adds the clock ids for the integrated DC-DC converter of the i.MX28: ref_xtal |\ | \ | \ +--+ | \| | ref_pll |\ |+--- CLK_DCDC | | \ +-+ | /| | | \ | | | / +--+ | +- DIV INT |--| / ref_gpmi | / | | |/ | / +-+ dcdc_sel |/dcdc_div dcdc_pll Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- .../devicetree/bindings/clock/imx28-clock.txt |3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/imx28-clock.txt b/Documentation/devicetree/bindings/clock/imx28-clock.txt index e6587af..3678e3c 100644 --- a/Documentation/devicetree/bindings/clock/imx28-clock.txt +++ b/Documentation/devicetree/bindings/clock/imx28-clock.txt @@ -76,6 +76,9 @@ clocks and IDs. usb0_phy62 usb1_phy63 enet_out64 + dcdc_pll65 + dcdc_div66 + dcdc_sel67 Examples: -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] clk: improve handling of orphan clocks
Hi Heiko, Heiko Stuebner he...@sntech.de hat am 22. April 2015 um 22:53 geschrieben: Using orphan clocks can introduce strange behaviour as they don't have rate information at all and also of course don't track [...] As this changes the behaviour for orphan clocks, it would of course benefit from more testing than on my Rockchip boards. To keep the recipent-list reasonable and not spam to much I selected one (the topmost) from the get_maintainer output of each drivers/clk entry. Hopefully some will provide Tested-by-tags :-) excuse me for my naive question, but what kind of tests do you expect (beside applying the patches)? Stefan Thanks Heiko -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] clk: Fix JSON output in debugfs
key/value pairs in a JSON object must be separated by a comma. After adding the properties accuracy and phase the JSON output of /sys/kernel/debug/clk/clk_dump is invalid. So add the missing commas to fix it. Fixes: 5279fc4 (clk: add clk accuracy retrieval support) Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/clk/clk.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index fa5a00e..693d2aa 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -233,8 +233,8 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) seq_printf(s, \%s\: { , c-name); seq_printf(s, \enable_count\: %d,, c-enable_count); seq_printf(s, \prepare_count\: %d,, c-prepare_count); - seq_printf(s, \rate\: %lu, clk_core_get_rate(c)); - seq_printf(s, \accuracy\: %lu, clk_core_get_accuracy(c)); + seq_printf(s, \rate\: %lu,, clk_core_get_rate(c)); + seq_printf(s, \accuracy\: %lu,, clk_core_get_accuracy(c)); seq_printf(s, \phase\: %d, clk_core_get_phase(c)); } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: add newline character after dumping all clocks
Hi, Felipe Balbi ba...@ti.com hat am 1. Mai 2015 um 16:48 geschrieben: clk_dump() will dump data about all clocks in JSON format, but it misses a newline character at the end of the JSON string. This patch adds that missing newline character. Signed-off-by: Felipe Balbi ba...@ti.com Acked-by: Stefan Wahren stefan.wah...@i2se.com Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] clk: Fix JSON output in debugfs
Stephen Boyd sb...@codeaurora.org hat am 1. Mai 2015 um 02:37 geschrieben: On 04/29, Stefan Wahren wrote: key/value pairs in a JSON object must be separated by a comma. After adding the properties accuracy and phase the JSON output of /sys/kernel/debug/clk/clk_dump is invalid. So add the missing commas to fix it. Fixes: 5279fc4 (clk: add clk accuracy retrieval support) Signed-off-by: Stefan Wahren stefan.wah...@i2se.com Hmph, this regression is old, v3.14 days. We probably ought to have a comment in here stating this should be JSON format. Applied to clk-next with the comment below squashed in. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] Regulator: mc34708: Add DT binding documentation
Hi Martin, Martin Fuzzey mfuz...@parkeon.com hat am 28. April 2015 um 16:17 geschrieben: Signed-off-by: Martin Fuzzey mfuz...@parkeon.com a commit message would be nice and the subject should specify dt-bindings instead of regulator subsystem. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] regulator: Add regulator driver for the MC34708 PMIC
Hi Martin, Martin Fuzzey mfuz...@parkeon.com hat am 28. April 2015 um 16:17 geschrieben: Add regulator driver and associated DT bindings for the regulators in the Freescale MC34708 PMIC. from my understanding this PMIC is used typically for i.MX50/53 SoCs. So in this case it would make sense to mention it here and CC this patch series also to the IMX maintainers. Btw git format-patch --cover-letter is your friend. Stefan ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Regulator: add driver for Freescale MC34708
Hi Martin, Martin Fuzzey mfuz...@parkeon.com hat am 28. April 2015 um 16:17 geschrieben: [...] diff --git a/drivers/regulator/mc34708-regulator.c b/drivers/regulator/mc34708-regulator.c new file mode 100644 index 000..b5ff727 --- /dev/null +++ b/drivers/regulator/mc34708-regulator.c @@ -0,0 +1,1266 @@ +/* + * Driver for regulators in the Fresscale MC34708/9 PMICs + * just a typo: Freescale Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] Regulator: mc34708: Add DT binding documentation
Hi Mark, Mark Brown broo...@kernel.org hat am 1. Mai 2015 um 13:01 geschrieben: On Fri, May 01, 2015 at 12:34:21PM +0200, Stefan Wahren wrote: a commit message would be nice and the subject should specify dt-bindings instead of regulator subsystem. No, it shouldn't. DT bindings for a subsystem go through the subsystem and the subsystem maintainer needs to see them to read them. sorry for my bad expressing. I tought the subject line should be something like: DT: add binding documentation for mc34708 regulator Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] dmaengine: bcm2835: Add slave dma support
Hi Noralf, Am 15.04.2015 um 11:56 schrieb Noralf Trønnes: Add slave transfer capability to BCM2835 dmaengine driver. This patch is pulled from the bcm2708-dmaengine driver in the Raspberry Pi repo. The work was done by Gellert Weisz. Tested with the bcm2835-mmc driver from the same repo. why not with the upstream kernel? Signed-off-by: Noralf Trønnes nor...@tronnes.org --- Gellert Weisz has ended his internship with Raspberry Pi Trading and was not available to sign off this patch. Changes from original code: Remove slave_id use. SDHCI_BCM_DMA_WAITS changed to BCM2835_DMA_WAIT_CYCLES. Use SZ_* macros instead of decimal values. Change MAX_LITE_TRANSFER from 32k to 64K - 1. Fix several whitespace issues. Lee Jones' comments in previous email to Piotr Król: Remove __func__ from dev_err message. Cleanup comments. Quick testing: Enable CONFIG_DMA_BCM2835 Apply patch: https://gist.github.com/notro/0c9184df1b43a793f006 drivers/dma/bcm2835-dma.c | 200 ++ 1 file changed, 186 insertions(+), 14 deletions(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index c92d6a7..4230aac 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -1,11 +1,10 @@ [...] + +static struct dma_async_tx_descriptor * +bcm2835_dma_prep_slave_sg(struct dma_chan *chan, + struct scatterlist *sgl, + unsigned int sg_len, + enum dma_transfer_direction direction, + unsigned long flags, void *context) +{ + struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); + enum dma_slave_buswidth dev_width; + struct bcm2835_desc *d; + dma_addr_t dev_addr; + struct scatterlist *sgent; + unsigned int es, sync_type; + unsigned int i, j, splitct, max_size; I think split_cnt would be better. + + if (!is_slave_direction(direction)) { + dev_err(chan-device-dev, direction not supported\n); + return NULL; + } + + if (direction == DMA_DEV_TO_MEM) { + dev_addr = c-cfg.src_addr; + dev_width = c-cfg.src_addr_width; + sync_type = BCM2835_DMA_S_DREQ; + } else { + dev_addr = c-cfg.dst_addr; + dev_width = c-cfg.dst_addr_width; + sync_type = BCM2835_DMA_D_DREQ; + } + + /* Bus width translates to the element size (ES) */ + switch (dev_width) { + case DMA_SLAVE_BUSWIDTH_4_BYTES: + es = BCM2835_DMA_DATA_TYPE_S32; Looks like es is never used. + break; + default: A dev_err() might be useful here. + return NULL; + } + + /* Allocate and setup the descriptor. */ + d = kzalloc(sizeof(*d), GFP_NOWAIT); + if (!d) + return NULL; + + d-dir = direction; + + if (c-ch = 8) /* LITE channel */ + max_size = MAX_LITE_TRANSFER; + else + max_size = MAX_NORMAL_TRANSFER; + + /* +* Store the length of the SG list in d-frames +* taking care to account for splitting up transfers +* too large for a LITE channel +*/ + d-frames = 0; + for_each_sg(sgl, sgent, sg_len, i) { + unsigned int len = sg_dma_len(sgent); + + d-frames += 1 + len / max_size; If it's correct this should be more intuitive: d-frames += len / max_size + 1; + } + + /* Allocate memory for control blocks */ + d-control_block_size = d-frames * sizeof(struct bcm2835_dma_cb); + d-control_block_base = dma_zalloc_coherent(chan-device-dev, + d-control_block_size, d-control_block_base_phys, + GFP_NOWAIT); + if (!d-control_block_base) { + kfree(d); + return NULL; + } + + /* +* Iterate over all SG entries, create a control block +* for each frame and link them together. +* Count the number of times an SG entry had to be split +* as a result of using a LITE channel +*/ + splitct = 0; + + for_each_sg(sgl, sgent, sg_len, i) { + dma_addr_t addr = sg_dma_address(sgent); + unsigned int len = sg_dma_len(sgent); + + for (j = 0; j len; j += max_size) { It should be possible to move declaration of j down here. + struct bcm2835_dma_cb *control_block = + d-control_block_base[i + splitct]; + + /* Setup addresses */ + if (d-dir == DMA_DEV_TO_MEM) { + control_block-info = BCM2835_DMA_D_INC | + BCM2835_DMA_D_WIDTH | + BCM2835_DMA_S_DREQ; + control_block-src =
Re: [PATCH] dmaengine: bcm2835: Add slave dma support
Hi Noralf, Am 17.04.2015 um 00:09 schrieb Noralf Trønnes: Den 15.04.2015 21:00, skrev Stefan Wahren: Hi Noralf, Am 15.04.2015 um 11:56 schrieb Noralf Trønnes: Add slave transfer capability to BCM2835 dmaengine driver. This patch is pulled from the bcm2708-dmaengine driver in the Raspberry Pi repo. The work was done by Gellert Weisz. Tested with the bcm2835-mmc driver from the same repo. why not with the upstream kernel? See my answer to Alexander Stein. i read the mail, but i'm still confused. Please let me paraphrase my last question: Is this patch testable with upstream kernel? It would be helpful to put those facts from the email to Alexander into the patch description. Please clarify the intension of your patch. Thanks Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: fix constraints debug output
In the case uV_offset is greater than 0 the debug output before is accidentally overwritten. So take care of the output count. Fixes: bf5892a8167e (regulator: Support voltage offsets to compensate for drops) CC: sta...@vger.kernel.org Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/regulator/core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 443eaab..9afa3af 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -801,7 +801,7 @@ static void print_constraints(struct regulator_dev *rdev) } if (constraints-uV_offset) - count += sprintf(buf, %dmV offset , + count += sprintf(buf + count, %dmV offset , constraints-uV_offset / 1000); if (constraints-min_uA constraints-max_uA) { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: fix constraints debug output
Hi Joe, Joe Perches j...@perches.com hat am 20. Mai 2015 um 22:35 geschrieben: On Wed, 2015-05-20 at 20:17 +, Stefan Wahren wrote: In the case uV_offset is greater than 0 the debug output before is accidentally overwritten. So take care of the output count. If you are going to take care, please change all of these sprintf calls to snprintf. thanks for pointing out. I will create a second version. But using scnprintf should be better here. The buf array may not be big enough [80] to hold the longest possible output string. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] regulator: core: fix constraints debug output
The line buffer for constraints debug isn't big enough to hold the output in all cases. So fix the possible buffer overflow by the usage of the scnprintf(). By the way this fixes that the debug output is overwritten when uV_offset is greater 0. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com CC: sta...@vger.kernel.org --- drivers/regulator/core.c | 43 --- 1 file changed, 24 insertions(+), 19 deletions(-) Changes in V2: - add fix for buffer overflow which has been noticed by Joe Perches diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index f698948..c19456e 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -780,58 +780,63 @@ static void print_constraints(struct regulator_dev *rdev) { struct regulation_constraints *constraints = rdev-constraints; char buf[80] = ; + size_t len = sizeof(buf) - 1; int count = 0; int ret; if (constraints-min_uV constraints-max_uV) { if (constraints-min_uV == constraints-max_uV) - count += sprintf(buf + count, %d mV , -constraints-min_uV / 1000); + count += scnprintf(buf + count, len - count, %d mV , + constraints-min_uV / 1000); else - count += sprintf(buf + count, %d -- %d mV , -constraints-min_uV / 1000, -constraints-max_uV / 1000); + count += scnprintf(buf + count, len - count, + %d -- %d mV , + constraints-min_uV / 1000, + constraints-max_uV / 1000); } if (!constraints-min_uV || constraints-min_uV != constraints-max_uV) { ret = _regulator_get_voltage(rdev); if (ret 0) - count += sprintf(buf + count, at %d mV , ret / 1000); + count += scnprintf(buf + count, len - count, + at %d mV , ret / 1000); } if (constraints-uV_offset) - count += sprintf(buf, %dmV offset , -constraints-uV_offset / 1000); + count += scnprintf(buf + count, len - count, %dmV offset , + constraints-uV_offset / 1000); if (constraints-min_uA constraints-max_uA) { if (constraints-min_uA == constraints-max_uA) - count += sprintf(buf + count, %d mA , -constraints-min_uA / 1000); + count += scnprintf(buf + count, len - count, %d mA , + constraints-min_uA / 1000); else - count += sprintf(buf + count, %d -- %d mA , -constraints-min_uA / 1000, -constraints-max_uA / 1000); + count += scnprintf(buf + count, len - count, + %d -- %d mA , + constraints-min_uA / 1000, + constraints-max_uA / 1000); } if (!constraints-min_uA || constraints-min_uA != constraints-max_uA) { ret = _regulator_get_current_limit(rdev); if (ret 0) - count += sprintf(buf + count, at %d mA , ret / 1000); + count += scnprintf(buf + count, len - count, + at %d mA , ret / 1000); } if (constraints-valid_modes_mask REGULATOR_MODE_FAST) - count += sprintf(buf + count, fast ); + count += scnprintf(buf + count, len - count, fast ); if (constraints-valid_modes_mask REGULATOR_MODE_NORMAL) - count += sprintf(buf + count, normal ); + count += scnprintf(buf + count, len - count, normal ); if (constraints-valid_modes_mask REGULATOR_MODE_IDLE) - count += sprintf(buf + count, idle ); + count += scnprintf(buf + count, len - count, idle ); if (constraints-valid_modes_mask REGULATOR_MODE_STANDBY) - count += sprintf(buf + count, standby); + count += scnprintf(buf + count, len - count, standby); if (!count) - sprintf(buf, no parameters); + scnprintf(buf, len, no parameters); rdev_dbg(rdev, %s\n, buf); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ
Re: [PATCH V2] regulator: core: fix constraints debug output
Mark Brown broo...@kernel.org hat am 9. Juni 2015 um 20:00 geschrieben: On Wed, Jun 03, 2015 at 09:56:29PM +, Stefan Wahren wrote: The line buffer for constraints debug isn't big enough to hold the output in all cases. So fix the possible buffer overflow by the usage of the scnprintf(). If we know how much output we might produce then why not extend the buffer so we don't need to cut it off? The defensive programming is good but that seems a better fix. I thought the intension was to keep the output in one line. If not i suggest to increase the size to 160 bytes, which is sufficient for all cases. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: fix constraints output buffer
The buffer for condtraints debug isn't big enough to hold the output in all cases. So fix this issue by increasing the buffer. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com CC: sta...@vger.kernel.org --- drivers/regulator/core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 9afa3af..53ed2d4 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -779,7 +779,7 @@ static int suspend_prepare(struct regulator_dev *rdev, suspend_state_t state) static void print_constraints(struct regulator_dev *rdev) { struct regulation_constraints *constraints = rdev-constraints; - char buf[80] = ; + char buf[160] = ; int count = 0; int ret; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] regulator: core: fix constraints output buffer
Mark Brown broo...@kernel.org hat am 10. Juni 2015 um 01:22 geschrieben: On Tue, Jun 09, 2015 at 08:09:42PM +, Stefan Wahren wrote: The buffer for condtraints debug isn't big enough to hold the output in all cases. So fix this issue by increasing the buffer. Applied, thanks. Will you send a rebased version of the scnprintf() patch as well? Why certainly! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V3] regulator: core: replace sprintf with scnprintf
In order to avoid potential overflows in print_constraints we better replace sprintf() with scnprintf(). Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/regulator/core.c | 43 --- 1 file changed, 24 insertions(+), 19 deletions(-) Changes in V3: - rebase on fix/core since uV_offset issue is already fixed Changes in V2: - add fix for buffer overflow which had been noticed by Joe Perches diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 53ed2d4..0efcbfb 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -780,58 +780,63 @@ static void print_constraints(struct regulator_dev *rdev) { struct regulation_constraints *constraints = rdev-constraints; char buf[160] = ; + size_t len = sizeof(buf) - 1; int count = 0; int ret; if (constraints-min_uV constraints-max_uV) { if (constraints-min_uV == constraints-max_uV) - count += sprintf(buf + count, %d mV , -constraints-min_uV / 1000); + count += scnprintf(buf + count, len - count, %d mV , + constraints-min_uV / 1000); else - count += sprintf(buf + count, %d -- %d mV , -constraints-min_uV / 1000, -constraints-max_uV / 1000); + count += scnprintf(buf + count, len - count, + %d -- %d mV , + constraints-min_uV / 1000, + constraints-max_uV / 1000); } if (!constraints-min_uV || constraints-min_uV != constraints-max_uV) { ret = _regulator_get_voltage(rdev); if (ret 0) - count += sprintf(buf + count, at %d mV , ret / 1000); + count += scnprintf(buf + count, len - count, + at %d mV , ret / 1000); } if (constraints-uV_offset) - count += sprintf(buf + count, %dmV offset , -constraints-uV_offset / 1000); + count += scnprintf(buf + count, len - count, %dmV offset , + constraints-uV_offset / 1000); if (constraints-min_uA constraints-max_uA) { if (constraints-min_uA == constraints-max_uA) - count += sprintf(buf + count, %d mA , -constraints-min_uA / 1000); + count += scnprintf(buf + count, len - count, %d mA , + constraints-min_uA / 1000); else - count += sprintf(buf + count, %d -- %d mA , -constraints-min_uA / 1000, -constraints-max_uA / 1000); + count += scnprintf(buf + count, len - count, + %d -- %d mA , + constraints-min_uA / 1000, + constraints-max_uA / 1000); } if (!constraints-min_uA || constraints-min_uA != constraints-max_uA) { ret = _regulator_get_current_limit(rdev); if (ret 0) - count += sprintf(buf + count, at %d mA , ret / 1000); + count += scnprintf(buf + count, len - count, + at %d mA , ret / 1000); } if (constraints-valid_modes_mask REGULATOR_MODE_FAST) - count += sprintf(buf + count, fast ); + count += scnprintf(buf + count, len - count, fast ); if (constraints-valid_modes_mask REGULATOR_MODE_NORMAL) - count += sprintf(buf + count, normal ); + count += scnprintf(buf + count, len - count, normal ); if (constraints-valid_modes_mask REGULATOR_MODE_IDLE) - count += sprintf(buf + count, idle ); + count += scnprintf(buf + count, len - count, idle ); if (constraints-valid_modes_mask REGULATOR_MODE_STANDBY) - count += sprintf(buf + count, standby); + count += scnprintf(buf + count, len - count, standby); if (!count) - sprintf(buf, no parameters); + scnprintf(buf, len, no parameters); rdev_dbg(rdev, %s\n, buf); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ config NVMEM_SUNXI_SID This driver can also be built as a module. If so, the module will be called eeprom-sunxi-sid. +config NVMEM_VF610_OCOTP + tristate VF610 SoCs OCOTP support + depends on SOC_VF610 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/9] Add simple NVMEM Framework via regmap.
Hi Srinivas, Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:07 geschrieben: [...] Device Tree: /* Provider */ qfprom: qfprom@0070 { ... /* Data cells */ tsens_calibration: calib@404 { reg = 0x404 0x10; }; tsens_calibration_bckp: calib_bckp@504 { reg = 0x504 0x11; bit-offset = 6; nbits = 128; }; pvs_version: pvs-version@6 { reg = 0x6 0x2 bit-offset = 7; nbits = 2; }; speed_bin: speed-bin@c{ reg = 0xc 0x1; bit-offset = 2; nbits = 3; }; ... }; userspace interface: binary file in /sys/class/nvmem/*/nvmem ex: hexdump /sys/class/nvmem/qfprom0/nvmem 000 * 0a0 db10 2240 e000 0c00 0c00 0c00 000 ... * 0001000 i want to port OCOTP driver for MXS, which hasn't MMIO. From my understanding hexdump would readout the complete register range defined in provider DT node. How can i achieve that hexdump only reads the data area within the register range? Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 8/9] nvmem: sunxi: Move the SID driver to the nvmem framework
Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:09 geschrieben: From: Maxime Ripard maxime.rip...@free-electrons.com Now that we have the nvmem framework, we can consolidate the common driver code. Move the driver to the framework, and hopefully, it will fix the sysfs file creation race. --- /dev/null +++ b/drivers/nvmem/sunxi-sid.c [...] + +static int sunxi_sid_write(void *context, const void *data, size_t count) +{ + /* Unimplemented */ + return 0; +} + I think it should be clear that the write operation isn't implemented. It's more important to know the function should make regmap_init happy. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 3/9] nvmem: Add nvmem_device based consumer apis.
Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:08 geschrieben: This patch adds read/write apis which are based on nvmem_device. It is common that the drivers like omap cape manager or qcom cpr driver to access bytes directly at particular offset in the eeprom and not from nvmem cell info in DT. These driver would need to get access to the nvmem directly, which is what these new APIS provide. [...] diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h index 123af62..f9acc43 100644 --- a/include/linux/nvmem-consumer.h +++ b/include/linux/nvmem-consumer.h @@ -15,6 +15,15 @@ struct device; /* consumer cookie */ struct nvmem_cell; +struct nvmem_device; + +struct nvmem_cell_info { + const char *name; + int offset; + int bytes; + int bit_offset; + int nbits; +}; #if IS_ENABLED(CONFIG_NVMEM) @@ -26,6 +35,21 @@ void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell); void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len); int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len); +/* direct nvmem device read/write interface */ +struct nvmem_device *nvmem_device_get(struct device *dev, const char *name); +struct nvmem_device *devm_nvmem_device_get(struct device *dev, + const char *name); +void nvmem_device_put(struct nvmem_device *nvmem); +void devm_nvmem_device_put(struct device *dev, struct nvmem_device *nvmem); +int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset, + size_t bytes, void *buf); +int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset, + size_t bytes, void *buf); Maybe i mixed something but those functions use the datatype unsigned int for offset and the offset in the structs use datatype int. Looks a little bit inconsistent. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
Hi Srinivas, sorry for the messed up indention. Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:08 geschrieben: [...] --- /dev/null +++ b/drivers/nvmem/Kconfig @@ -0,0 +1,10 @@ +menuconfig NVMEM + tristate NVMEM Support + select REGMAP + help + Support for NVMEM devices. + + This framework is designed to provide a generic interface to NVMEM + from both the Linux Kernel and the userspace. In order to avoid confusion it would be good to write here what does NVMEM mean. + + If unsure, say no. diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile new file mode 100644 index 000..6df2c69 --- /dev/null +++ b/drivers/nvmem/Makefile @@ -0,0 +1,6 @@ +# +# Makefile for nvmem drivers. +# + +obj-$(CONFIG_NVMEM) += nvmem_core.o +nvmem_core-y := core.o diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c new file mode 100644 index 000..17a826d --- /dev/null +++ b/drivers/nvmem/core.c @@ -0,0 +1,398 @@ +/* + * nvmem framework core. + * + * Copyright (C) 2015 Srinivas Kandagatla srinivas.kandaga...@linaro.org + * Copyright (C) 2013 Maxime Ripard maxime.rip...@free-electrons.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 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/device.h +#include linux/nvmem-provider.h +#include linux/export.h +#include linux/fs.h +#include linux/idr.h +#include linux/init.h +#include linux/regmap.h +#include linux/module.h +#include linux/of.h +#include linux/slab.h Sorting alphabetically would by nice + [...] +/** + * nvmem_register() - Register a nvmem device for given nvmem_config. + * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem + * + * @config: nvmem device configuration with which nvmem device is created. + * + * The return value will be an ERR_PTR() on error or a valid pointer + * to nvmem_device. + */ + +struct nvmem_device *nvmem_register(struct nvmem_config *config) +{ + struct nvmem_device *nvmem; + struct regmap *rm; + int rval; + + if (!config-dev) + return ERR_PTR(-EINVAL); + + rm = dev_get_regmap(config-dev, NULL); + if (!rm) { + dev_err(config-dev, Regmap not found\n); + return ERR_PTR(-EINVAL); + } + + nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL); + if (!nvmem) + return ERR_PTR(-ENOMEM); + + nvmem-id = ida_simple_get(nvmem_ida, 0, 0, GFP_KERNEL); + if (nvmem-id 0) { + kfree(nvmem); + return ERR_PTR(nvmem-id); + } + + nvmem-regmap = rm; + nvmem-owner = config-owner; + nvmem-stride = regmap_get_reg_stride(rm); + nvmem-word_size = regmap_get_val_bytes(rm); + nvmem-size = regmap_get_max_register(rm) + nvmem-stride; + nvmem-dev.class = nvmem_class; + nvmem-dev.parent = config-dev; + nvmem-dev.of_node = config-dev-of_node; + dev_set_name(nvmem-dev, %s%d, + config-name ? : nvmem, config-id); + + nvmem-read_only = nvmem-dev.of_node ? + of_property_read_bool(nvmem-dev.of_node, + read-only) : + config-read_only; + + device_initialize(nvmem-dev); + + dev_dbg(nvmem-dev, Registering nvmem device %s\n, + dev_name(nvmem-dev)); + + rval = device_add(nvmem-dev); + if (rval) { + ida_simple_remove(nvmem_ida, nvmem-id); + kfree(nvmem); + return ERR_PTR(rval); + } + + /* update sysfs attributes */ + if (nvmem-read_only) + sysfs_update_group(nvmem-dev.kobj, nvmem_bin_ro_group); + + if (config-cells) + nvmem_add_cells(nvmem, config); I think this would be a better place for the debug message from above. Additionally we could add the cell count and so on. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/9] nvmem: Add bindings for simple nvmem framework
Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:08 geschrieben: This patch adds bindings for simple nvmem framework which allows nvmem consumers to talk to nvmem providers to get access to nvmem cell data. Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com [Maxime Ripard: intial version of eeprom framework] Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- Documentation/devicetree/bindings/nvmem/nvmem.txt | 85 +++ 1 file changed, 85 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt new file mode 100644 index 000..d1a37e7 --- /dev/null +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt @@ -0,0 +1,85 @@ += NVMEM(Non Volatile Memory) Data Device Tree Bindings = + +This binding is intended to represent the location of hardware +configuration data stored in NVMEMs like eeprom, efuses and so on. + +On a significant proportion of boards, the manufacturer has stored +some data on NVMEM, for the OS to be able to retrieve these information +and act upon it. Obviously, the OS has to know about where to retrieve +these data from, and where they are stored on the storage device. + +This document is here to document this. + += Data providers = +Contains bindings specific to provider drivers and data cells as children +of this node. + +Optional properties: + read-only: Mark the provider as read only. + += Data cells = +These are the child nodes of the provider which contain data cell +information like offset and size in nvmem provider. + +Required properties: +reg: specifies the offset in byte within that storage device, start bit + in the byte and the length in bits of the data we care about. Is the second parameter really in bits, not bytes? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 2/9] nvmem: Add a simple NVMEM framework for consumers
Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 23. Juni 2015 um 01:08 geschrieben: [...] --- /dev/null +++ b/include/linux/nvmem-consumer.h @@ -0,0 +1,75 @@ +/* + * nvmem framework consumer. + * + * Copyright (C) 2015 Srinivas Kandagatla srinivas.kandaga...@linaro.org + * Copyright (C) 2013 Maxime Ripard maxime.rip...@free-electrons.com + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed as is without any + * warranty of any kind, whether express or implied. + */ + +#ifndef _LINUX_NVMEM_CONSUMER_H +#define _LINUX_NVMEM_CONSUMER_H + +struct device; Do we need forward declaration of struct device_node too? +/* consumer cookie */ [...] +} +#endif /* CONFIG_NVMEM */ + +#if IS_ENABLED(CONFIG_NVMEM) IS_ENABLED(CONFIG_OF) +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, + const char *name); +#else +static inline struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, + const char *name) +{ + return ERR_PTR(-ENOSYS); +} +#endif /* CONFIG_NVMEM CONFIG_OF */ + +#endif /* ifndef _LINUX_NVMEM_CONSUMER_H */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hi Sanchayan, Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: On 15-06-23 21:31:41, Stefan Wahren wrote: Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ config NVMEM_SUNXI_SID This driver can also be built as a module. If so, the module will be called eeprom-sunxi-sid. +config NVMEM_VF610_OCOTP + tristate VF610 SoCs OCOTP support + depends on SOC_VF610 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the select REGMAP_MMIO and it works just fine. - Sanchayan. sorry for the confusion. My question refers to the whole driver implementation not only to the REGMAP_MMIO. According to Vybrid Reference Manual F-Series Document Number: VYBRIDRM Rev 7, 06/2014 35.5 OCOTP memory map/register definition the memory region is organized in control and shadow registers. I'm very sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. It possible that it works in your case. But in the case the lock bits are set the driver won't work correctly. Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 2/2] nvmem: Add Vybrid OCOTP and OCROM support
Hi Srinivas, Am 24.06.2015 um 12:45 schrieb maitysancha...@gmail.com: Hello Stefan, On 15-06-24 11:56:14, Stefan Wahren wrote: Hi Sanchayan, Am 24.06.2015 um 07:19 schrieb maitysancha...@gmail.com: On 15-06-23 21:31:41, Stefan Wahren wrote: Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 23. Juni 2015 um 15:44 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) and On Chip ROM (OCROM) support. On Vybrid OCOTP contain data like SoC ID, MAC address and OCROM has the revision ID. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 11 + drivers/nvmem/Makefile | 2 ++ drivers/nvmem/vf610-ocotp.c | 60 + 3 files changed, 73 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..557c1e0 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,15 @@ config NVMEM_SUNXI_SID This driver can also be built as a module. If so, the module will be called eeprom-sunxi-sid. +config NVMEM_VF610_OCOTP + tristate VF610 SoCs OCOTP support + depends on SOC_VF610 + select REGMAP_MMIO how do you come to the conclusion that Vybrid On-Chip OTP is accessable via MMIO? Frankly speaking I just changed the naming conventions and followed the qfrom and sunxi sid examples in Srinivas's patches. I just tested it without the select REGMAP_MMIO and it works just fine. - Sanchayan. sorry for the confusion. My question refers to the whole driver implementation not only to the REGMAP_MMIO. According to Vybrid Reference Manual F-Series Document Number: VYBRIDRM Rev 7, 06/2014 35.5 OCOTP memory map/register definition the memory region is organized in control and shadow registers. I'm very sceptical that using REGMAP_MMIO is the right way for accessing the OCOTP. Sorry I am not well versed with the regmap stuff. Can you please tell me why REGMAP_MMIO is not the right way for accessing the OCOTP? i think the implementation of OCOTP driver should be more like this: https://git.linaro.org/people/srinivas.kandagatla/linux.git/commit/b4c3ad253747767511233687436f20144e850d67 It uses REGMAP with a specialized read function. If this is not the right way, I assume you are referring to the procedures in section 35.3.1.5 and 35.3.1.6 for reading and writing from these areas? Yes. I think writing isn't needed. But the read function should check at least for OCOTP_CTRL[BUSY] and OCOTP_CTRL[ERROR]. If one of the bits is set, the read function should return with an error. This is the same behavior of my OCOTP driver for MXS platform. Unfortunately i haven't push the driver to my github account. It possible that it works in your case. But in the case the lock bits are set the driver won't work correctly. As such the previous implementations were very different. Currently I only expect to provide a way for users to read the SoC ID from the OCOTP block. My understanding was even if the lock bits are set, reading from the registers will return 0xBADABADA. For example, currently for three locations I see this from ocotp block * 080 bada bada bada bada bada bada bada bada * 500 bada bada bada bada bada bada bada bada * c80 bada bada bada bada bada bada bada bada * - Sanchayan. Until somebody comes to the idea to fetch the MAC address from the OCOTP block ... How about doing it right at this point, instead of fixing it later? Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/9] Add simple NVMEM Framework via regmap.
Hi Srinivas, Am 24.06.2015 um 11:46 schrieb Srinivas Kandagatla: On 23/06/15 20:47, Stefan Wahren wrote: 0001000 i want to port OCOTP driver for MXS, which hasn't MMIO. From my understanding That's cool. hexdump would readout the complete register range defined in provider DT node. How can i achieve that hexdump only reads the data area within the register range? If the question is just about hexdump, then hexdump itself can read file from given offset and size. yes, this is my question at first. Let me show the difference between the current implementation and my expectations as a user. $ hexdump /sys/class/nvmem/mxs-ocotp/nvmem Current implementation: dump the complete register range defined in DT User expectation: dump only the data from OCOTP block Let me explain it for i.MX28 OCOTP 0x8002c000 // Start of OCOTP register block (defined in DT) 0x8002c020 // First data register 0x8002c290 // Last data register 0x8002dfff // End of OCOTP register block (defined in DT) My knowledge about regmap is limited, but how can i achieve that hexdump give me only the data registers? From my understanding this should be handled in regmap and not in the read function. Are my expectations about the raw access wrong? But I believe the real question is How can we dump each nvmem cell independently In one of my replies I mentioned that am planning to add sysfs entries under /sys/class/nvmem/provider/cells/ ex: for qfprom tsens calibration it would look like: $ hexdump /sys/class/nvmem/qfprom0/cells/tsens_calibration 000 e000 0c00 0c00 0c00 ... Is that what you guys are looking for? That would be nice, too :-) --srini Stefan TIA Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/9] Add simple NVMEM Framework via regmap.
Hi Srinivas, Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 24. Juni 2015 um 15:03 geschrieben: On 24/06/15 13:30, Stefan Wahren wrote: If the question is just about hexdump, then hexdump itself can read file from given offset and size. yes, this is my question at first. Let me show the difference between the current implementation and my expectations as a user. $ hexdump /sys/class/nvmem/mxs-ocotp/nvmem Current implementation: dump the complete register range defined in DT Its dumping the range which is specified in the provider regmap. If the requirement is to dump only particular range, this has to be made explicit while creating regmap, which is to specify the base address to start from First data register and max_register to be Last data register - First data register i know about max_register, but i can't find the base address in regmap_config. Do you mean struct regmap_access_table *rd_table ? User expectation: dump only the data from OCOTP block Let me explain it for i.MX28 OCOTP 0x8002c000 // Start of OCOTP register block (defined in DT) 0x8002c020 // First data register 0x8002c290 // Last data register 0x8002dfff // End of OCOTP register block (defined in DT) My knowledge about regmap is limited, but how can i achieve that hexdump give me only the data registers? From my understanding this should be handled in regmap and not in the read function. Setup the base and regmap_config correctly in the provider driver before calling regmap_init_mmio(). Let me know if you need more details. Yes, please. Stefan --srini Are my expectations about the raw access wrong? ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] Add the efuse driver on rockchip platform
Hi Srinivas, Am 16.06.2015 um 12:54 schrieb Srinivas Kandagatla: On 16/06/15 11:06, Caesar Wang wrote: Hi Srinivas, 在 2015年06月16日 17:21, Srinivas Kandagatla 写道: Hi Stefan, On 16/06/15 09:52, Stefan Wahren wrote: Hi Caesar, [add Maxime and Srinivas] Am 16.06.2015 um 09:27 schrieb Caesar Wang: The original driver is uploaded by Jianqun. Here is his patchs: https://patchwork.kernel.org/patch/5410341/ https://patchwork.kernel.org/patch/5410351/ Jianqun, nevermind! I check-pick it and re-upload the driver for the upstream. e.g.: Tested by on minnie board.(kernel-4.1-rc8) cd /sys/devices/platform/ffb4.efuse localhost ffb4.efuse # cat cpu_leakage_show cpu_version_show The results: 19 2 Changes in v2: - Change the document decription. - Move the efuse driver into driver/soc/vendor. - update the efuse driver. - Add the dts node on RK3288. i want to mention that there is a upcoming new framework suitable for efuse drivers: https://lkml.org/lkml/2015/5/21/643 Unfortunately i don't know the current development state. Currently this framework is used by atleast 3 drivers(qcom-tsens, qcom-cpr, begel-bone-cape manager) which are still floating in the mailing list. I was hoping that these 3 users would getback with tested-by.. which did not happen for last 3-4 weeks. I would appreciate, If you could try framework too, and let me know. yes i work on OCOTP driver for MXS platform and i will try ... int rockchip_efuse_reg_read(void *context, unsigned int reg, unsigned int *val) { /* efuse specific read sequence */ ... } I will need a specific read sequence too. Sorry for these newbie questions: What data structure does context points to for this reg_read opteration? Do we need range checking of reg or is it handled by the framework? Are there any limitation for reg_read regarding sleeping or locking operations? In case of a read only driver, is everything handle by devicetree or do we need an empty write operation? Best regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] watchdog: bcm2835: Add poweroff code for the Raspberry Pi
Hi Noralf, Am 09.06.2015 um 12:21 schrieb Noralf Trønnes: This adds a new poweroff function to the watchdog driver for the Raspberry Pi. Currently poweroff/halt results in a reboot. [...] +static void rpi_power_off(void) +{ + struct device_node *np = + of_find_compatible_node(NULL, NULL, brcm,raspberrypi-pm-wdt); + struct platform_device *pdev = of_find_device_by_node(np); + struct bcm2835_wdt *wdt = platform_get_drvdata(pdev); + u32 val; + + val = readl_relaxed(wdt-base + PM_RSTS); do you think it's safe here to assume wdt could never be NULL? May be it's necessary to send the series to the watchdog / bcm2835 maintainers to get more feedback. Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/3] Add the efuse driver on rockchip platform
Hi Caesar, [add Maxime and Srinivas] Am 16.06.2015 um 09:27 schrieb Caesar Wang: The original driver is uploaded by Jianqun. Here is his patchs: https://patchwork.kernel.org/patch/5410341/ https://patchwork.kernel.org/patch/5410351/ Jianqun, nevermind! I check-pick it and re-upload the driver for the upstream. e.g.: Tested by on minnie board.(kernel-4.1-rc8) cd /sys/devices/platform/ffb4.efuse localhost ffb4.efuse # cat cpu_leakage_show cpu_version_show The results: 19 2 Changes in v2: - Change the document decription. - Move the efuse driver into driver/soc/vendor. - update the efuse driver. - Add the dts node on RK3288. i want to mention that there is a upcoming new framework suitable for efuse drivers: https://lkml.org/lkml/2015/5/21/643 Unfortunately i don't know the current development state. Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v6 3/3] drivers: nvmem: Add Vybrid OCOTP support
Hi Sanchayan, Sanchayan Maity maitysancha...@gmail.com hat am 29. Juni 2015 um 13:22 geschrieben: The patch adds support for the On Chip One Time Programmable Peripheral (OCOTP) on the Vybrid platform. please provide a changelog in your cover letter and a new version after changes. I think it's important to note that the driver only support read-only access. Signed-off-by: Sanchayan Maity maitysancha...@gmail.com --- drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/vf610-ocotp.c | 250 3 files changed, 262 insertions(+) create mode 100644 drivers/nvmem/vf610-ocotp.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 17f1a57..84c830d 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -33,4 +33,14 @@ config NVMEM_SUNXI_SID This driver can also be built as a module. If so, the module will be called eeprom-sunxi-sid. +config NVMEM_VF610_OCOTP + tristate VF610 SoCs OCOTP support + depends on SOC_VF610 + help + This is a driver for the 'OCOTP' available on various Vybrid + devices. I don't know much about Vybrid. But this driver is specific for VF610, isn't it? + + This driver can also be built as a module. If so, the module + will be called nvmem-vf610-ocotp. + endif diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index cc46791..a9ed113 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -11,3 +11,5 @@ obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o nvmem_qfprom-y := qfprom.o obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o nvmem-sunxi-sid-y := sunxi-sid.o +obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o +nvmem-vf610-ocotp-y := vf610-ocotp.o diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c new file mode 100644 index 000..b7a782c --- /dev/null +++ b/drivers/nvmem/vf610-ocotp.c @@ -0,0 +1,250 @@ +/* + * Copyright (C) 2015 Sanchayan Maity sanchayan.ma...@toradex.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 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; 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.h +#include linux/delay.h +#include linux/device.h +#include linux/nvmem-provider.h +#include linux/io.h +#include linux/module.h +#include linux/of.h +#include linux/platform_device.h +#include linux/regmap.h +#include linux/slab.h + +/* OCOTP Register Offsets */ +#define OCOTP_CTRL_REG 0x00 +#define OCOTP_CTRL_SET 0x04 +#define OCOTP_CTRL_CLR 0x08 +#define OCOTP_TIMING 0x10 +#define OCOTP_DATA 0x20 +#define OCOTP_READ_CTRL_REG 0x30 +#define OCOTP_READ_FUSE_DATA 0x40 + +/* OCOTP Register bits and masks */ +#define OCOTP_CTRL_WR_UNLOCK 16 +#define OCOTP_CTRL_WR_UNLOCK_KEY 0x3E77 +#define OCOTP_CTRL_WR_UNLOCK_MASK 0x +#define OCOTP_CTRL_ADDR 0 +#define OCOTP_CTRL_ADDR_MASK 0x7F +#define OCOTP_CTRL_RELOAD_SHADOWS (0x1 10) +#define OCOTP_CTRL_ERROR (0x1 9) +#define OCOTP_CTRL_BUSY (0x1 8) You can use the BIT() macro here. + +#define OCOTP_TIMING_STROBE_READ 16 +#define OCOTP_TIMING_STROBE_READ_MASK 0x003F +#define OCOTP_TIMING_RELAX 12 +#define OCOTP_TIMING_RELAX_MASK 0xF000 +#define OCOTP_TIMING_STROBE_PROG 0 +#define OCOTP_TIMING_STROBE_PROG_MASK 0x0FFF + +#define OCOTP_READ_CTRL_READ_FUSE 0x1 + +#define VF610_OCOTP_TIMEOUT 10 + +#define BF(value, field) (((value) field) field##_MASK) + +#define DEF_RELAX 20 + +struct vf610_ocotp_dev { + void __iomem *base; + struct clk *clk; + struct device *dev; + struct resource *res; + struct regmap *regmap; + struct nvmem_device *nvmem; +}; Some member of this struct seems unnecessary to me. Please remove the unused one. + +static int ocotp_timing; How about storing the timings in struct above? + +static u8 valid_fuse_addr[] = { + 0x00, 0x01, 0x02, 0x04, 0x0F, 0x20, 0x21, 0x22, 0x23, 0x24, + 0x25, 0x26, 0x27, 0x28, 0x2F, 0x38, 0x39, 0x3A, 0x3B, 0x3C, + 0x3D, 0x3E, 0x3F +}; const? + +static int vf610_ocotp_wait_busy(void __iomem *base) +{ + int timeout = VF610_OCOTP_TIMEOUT; + + while ((readl(base) OCOTP_CTRL_BUSY) --timeout) + udelay(10); + + if (!timeout) { + writel(OCOTP_CTRL_ERROR, base + OCOTP_CTRL_CLR); + return -ETIMEDOUT; + } + + udelay(10); + + return 0; +} + +static int vf610_ocotp_calculate_timing(struct vf610_ocotp_dev *ocotp_dev) +{ + u32 clk_rate; + u32 relax, strobe_read, strobe_prog; + u32 timing; + + clk_rate = clk_get_rate(ocotp_dev-clk); If clk_get_rate() fails, then we should abort with a warning. + + relax = clk_rate / (10 / DEF_RELAX) - 1; + strobe_prog
Re: [PATCH RFC] eeprom: at24: extend driver to plug into the NVMEM framework
Hi Andrew, Andrew Lunn and...@lunn.ch hat am 16. August 2015 um 04:54 geschrieben: Add a read only regmap for accessing the EEPROM, and then use that with the NVMEM framework. Signed-off-by: Andrew Lunn and...@lunn.ch --- drivers/misc/eeprom/at24.c | 65 ++ 1 file changed, 65 insertions(+) diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 2d3db81be099..0e80c0c09d4e 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -22,6 +22,8 @@ #include linux/jiffies.h #include linux/of.h #include linux/i2c.h +#include linux/nvmem-provider.h +#include linux/regmap.h shouldn't the dependancies in Kconfig updated (depends on REGMAP) too? #include linux/platform_data/at24.h /* @@ -69,6 +71,10 @@ struct at24_data { unsigned write_max; unsigned num_addresses; + struct regmap_config regmap_config; + struct nvmem_config nvmem_config; + struct nvmem_device *nvmem; + /* * Some chips tie up multiple I2C addresses; dummy devices reserve * them for us, and we'll use them with SMBus calls. @@ -471,6 +477,34 @@ static ssize_t at24_macc_write(struct memory_accessor *macc, const char *buf, /*-*/ +/* + * Provide a regmap interface, which is registered with the NVMEM + * framework +*/ +static int at24_regmap_read(void *context, const void *reg, size_t reg_size, + void *val, size_t val_size) +{ + struct at24_data *at24 = context; + off_t offset = *(u32 *)reg; + + return at24_read(at24, val, offset, val_size); +} + +static int at24_regmap_write(void *context, const void *data, size_t count) +{ + struct at24_data *at24 = context; + + return at24_write(at24, data, 0, count); Since the patch only provides read only support this function could return 0. Regards Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] eeprom: at24: extend driver to plug into the NVMEM framework
Hi Andrew, Andrew Lunn and...@lunn.ch hat am 16. August 2015 um 15:11 geschrieben: On Sun, Aug 16, 2015 at 10:28:06AM +0200, Stefan Wahren wrote: Hi Andrew, Andrew Lunn and...@lunn.ch hat am 16. August 2015 um 04:54 geschrieben: Add a read only regmap for accessing the EEPROM, and then use that with the NVMEM framework. Signed-off-by: Andrew Lunn and...@lunn.ch --- drivers/misc/eeprom/at24.c | 65 ++ 1 file changed, 65 insertions(+) diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 2d3db81be099..0e80c0c09d4e 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -22,6 +22,8 @@ #include linux/jiffies.h #include linux/of.h #include linux/i2c.h +#include linux/nvmem-provider.h +#include linux/regmap.h shouldn't the dependancies in Kconfig updated (depends on REGMAP) too? Hi Stefan This is why the patch is RFC. REGMAP has stub implementations for when it is not. NVMEM also has stub implementations. NVMEM does however select REGMAP. So it should be possible to compile this driver without NVMEM support. Hopefully 0day will find my git branch and compile it in different ways to see if i've got this right. you are right. As part of RFC, is this O.K? Another question which spring to mind is, do we want the eeprom to be in /sys twice, the old and the new way? Backwards compatibility says the old must stay. Do we want a way to suppress the new? Or should we be going as far as refractoring the code into a core library, and two wrapper drivers, old and new? I think these are questions for the framework maintainers. +static int at24_regmap_write(void *context, const void *data, size_t count) +{ + struct at24_data *at24 = context; + + return at24_write(at24, data, 0, count); Since the patch only provides read only support this function could return 0. Humm, the comment is out of date. Regmap does not work too well without a write method. So i ended up implementing it. But it has hardly been tested, where as i have hammered on read. I think you didn't understand my comment. I know the write function is necessary, but calling at24_write() instead of simply returning 0 does make no sense to me. Regards Stefan Thanks Andrew -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/7] drm/vc4: Add devicetree bindings for VC4.
Hi Eric, Am 18.08.2015 um 23:54 schrieb Eric Anholt: VC4 is the GPU (display and 3D) subsystem present on the 2835 and some other Broadcom SoCs. This binding follows the model of msm, imx, sti, and others, where there is a subsystem node for the whole GPU, with nodes for the individual HW components within it. Signed-off-by: Eric Anholt e...@anholt.net --- v2: Extend the commit message, fix several nits from Stephen Warren. .../devicetree/bindings/gpu/brcm,bcm-vc4.txt | 79 ++ 1 file changed, 79 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt new file mode 100644 index 000..1b9fedc --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt @@ -0,0 +1,79 @@ +Broadcom VC4 GPU + +The VC4 device present on the Raspberry Pi includes a display system +with HDMI output and the HVS scaler for compositing display planes. + +Required properties for VC4: +- compatible: Should be brcm,vc4 +- crtcs: List of phandles of pixelvalve scanout engines +- hvss:List of phandles of HVS video scalers +- encoders:List of phandles of output encoders (HDMI, SDTV) + +Required properties for Pixel Valve: +- compatible: Should be brcm,vc4-pixelvalve +- reg: Physical base address and length of the PV's registers +- interrupts: The interrupt number + See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt + +Required properties for HVS: +- compatible: Should be brcm,vc4-hvs +- reg: Physical base address and length of the HVS's registers +- interrupts: The interrupt number + See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt + +Required properties for HDMI +- compatible: Should be brcm,vc4-hdmi +- reg: Physical base address and length of the two register ranges + (HDMI and HD, in that order) +- interrupts: The interrupt numbers + See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt +- ddc: phandle of the I2C controller used for DDC EDID probing +- crtc:phandle to the pixelvalve CRTC the HDMI encoder is attached to + +Optional properties for HDMI: +- hpd-gpio:The GPIO pin for HDMI hotplug detect (if it doesn't appear + as an interrupt/status bit in the HDMI controller + itself). See bindings/pinctrl/brcm,bcm2835-gpio.txt + +Example: +pv0: brcm,vc4-pixelvalve@7e206000 { AFAIK the DT node name should describe the function and usually don't have a vendor prefix. Here some possible suggestions without deeper knowledge of the hardware function: pv0: pixelvalve + compatible = brcm,vc4-pixelvalve; + reg = 0x7e206000 0x100; + interrupts = 2 13; /* pwa2 */ +}; + +pv1: brcm,vc4-pixelvalve@7e207000 { pv1: pixelvalve + compatible = brcm,vc4-pixelvalve; + reg = 0x7e207000 0x100; + interrupts = 2 14; /* pwa1 */ +}; + +pv2: brcm,vc4-pixelvalve@7e807000 { pv2: pixelvalve + compatible = brcm,vc4-pixelvalve; + reg = 0x7e807000 0x100; + interrupts = 2 10; /* pixelvalve */ +}; + +hvs: brcm,hvs@7e40 { hvs: hvs + compatible = brcm,vc4-hvs; + reg = 0x7e40 0x6000; + interrupts = 2 1; +}; + +hdmi: brcm,vc4-hdmi@7e902000 { hdmi: hdmi + compatible = brcm,vc4-hdmi; + reg = 0x7e902000 0x600, + 0x7e808000 0x100; + interrupts = 2 8, 2 9; + ddc = i2c2; + hpd-gpio = gpio 46 GPIO_ACTIVE_HIGH; + crtc = pv2; +}; + +vc4: vc4@0x7e4c { vc4: gpu Regards Stefan + compatible = brcm,vc4; + + crtcs = pv0, pv1, pv2; + encoders = hdmi; + hvss = hvs; +}; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/7] drm/vc4: Add KMS support for Raspberry Pi.
Hi Eric, only a few nits. Am 18.08.2015 um 23:54 schrieb Eric Anholt: This is the start of a full VC4 driver. Right now this just supports configuring the display using a pre-existing video mode (because changing the pixel clock isn't available yet, and doesn't work when it is). However, this is enough for fbcon and bringing up X using xf86-video-modesetting. Signed-off-by: Eric Anholt e...@anholt.net --- v2: Drop FB_HELPER select thanks to Archit's patches. Do manual init ordering instead of using the .load hook. Structure registration more like tegra's, but still using the typical component code. Drop no-op hooks for atomic_begin and mode_fixup() now that they're optional. Drop sentinel in Makefile. Fix minor style nits I noticed on another reread. drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/vc4/Kconfig | 13 + drivers/gpu/drm/vc4/Makefile | 17 + drivers/gpu/drm/vc4/vc4_bo.c | 52 drivers/gpu/drm/vc4/vc4_crtc.c| 565 ++ drivers/gpu/drm/vc4/vc4_debugfs.c | 38 +++ drivers/gpu/drm/vc4/vc4_drv.c | 271 drivers/gpu/drm/vc4/vc4_drv.h | 120 drivers/gpu/drm/vc4/vc4_hdmi.c| 633 ++ drivers/gpu/drm/vc4/vc4_hvs.c | 161 ++ drivers/gpu/drm/vc4/vc4_kms.c | 84 + drivers/gpu/drm/vc4/vc4_plane.c | 320 +++ drivers/gpu/drm/vc4/vc4_regs.h| 562 + 14 files changed, 2839 insertions(+) create mode 100644 drivers/gpu/drm/vc4/Kconfig create mode 100644 drivers/gpu/drm/vc4/Makefile create mode 100644 drivers/gpu/drm/vc4/vc4_bo.c create mode 100644 drivers/gpu/drm/vc4/vc4_crtc.c create mode 100644 drivers/gpu/drm/vc4/vc4_debugfs.c create mode 100644 drivers/gpu/drm/vc4/vc4_drv.c create mode 100644 drivers/gpu/drm/vc4/vc4_drv.h create mode 100644 drivers/gpu/drm/vc4/vc4_hdmi.c create mode 100644 drivers/gpu/drm/vc4/vc4_hvs.c create mode 100644 drivers/gpu/drm/vc4/vc4_kms.c create mode 100644 drivers/gpu/drm/vc4/vc4_plane.c create mode 100644 drivers/gpu/drm/vc4/vc4_regs.h [...] +static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc) +{ + struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + struct drm_crtc_state *state = crtc-state; + struct drm_display_mode *mode = state-adjusted_mode; + u32 vactive = (mode-vdisplay + ((mode-flags DRM_MODE_FLAG_INTERLACE) ? 1 : 0)); + u32 format = PV_CONTROL_FORMAT_24; + bool debug_dump_regs = false; + + if (debug_dump_regs) { + DRM_INFO(CRTC %d regs before:\n, drm_crtc_index(crtc)); + vc4_crtc_dump_regs(vc4_crtc); + } + + /* This is where we would set the pixel clock. */ + + /* Reset the PV fifo. */ + CRTC_WRITE(PV_CONTROL, 0); + CRTC_WRITE(PV_CONTROL, PV_CONTROL_FIFO_CLR | PV_CONTROL_EN); + CRTC_WRITE(PV_CONTROL, 0); + + CRTC_WRITE(PV_HORZA, + VC4_SET_FIELD(mode-htotal - mode-hsync_end, +PV_HORZA_HBP) | + VC4_SET_FIELD(mode-hsync_end - mode-hsync_start, +PV_HORZA_HSYNC)); + CRTC_WRITE(PV_HORZB, + VC4_SET_FIELD(mode-hsync_start - mode-hdisplay, +PV_HORZB_HFP) | + VC4_SET_FIELD(mode-hdisplay, PV_HORZB_HACTIVE)); + + CRTC_WRITE(PV_VERTA, + VC4_SET_FIELD(mode-vtotal - mode-vsync_end, +PV_VERTA_VBP) | + VC4_SET_FIELD(mode-vsync_end - mode-vsync_start, +PV_VERTA_VSYNC)); + CRTC_WRITE(PV_VERTB, + VC4_SET_FIELD(mode-vsync_start - mode-vdisplay, +PV_VERTB_VFP) | + VC4_SET_FIELD(vactive, PV_VERTB_VACTIVE)); + if (mode-flags DRM_MODE_FLAG_INTERLACE) { + /* Write PV_VERTA_EVEN/VERTB_EVEN */ + } checkpatch complains here. Is this intended to have only a comment? Is it a TODO? [...] --- /dev/null +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2015 Broadcom + * + * 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. + */ + +#include drmP.h +#include drm_gem_cma_helper.h + +struct vc4_dev { + struct drm_device *dev; + + struct vc4_hdmi *hdmi; + struct vc4_hvs *hvs; + struct vc4_crtc *crtc[3]; +}; + +static inline struct vc4_dev * +to_vc4_dev(struct drm_device *dev) +{ + return (struct vc4_dev *)dev-dev_private; +} + +struct vc4_bo { + struct drm_gem_cma_object base; +}; + +static inline struct vc4_bo * +to_vc4_bo(struct drm_gem_object *bo) +{ + return (struct
[PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection
Since bits_per_word isn't usually checked during SPI setup the 9-bit support must be checked manually. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/staging/fbtft/fbtft-core.c |7 +++ drivers/staging/fbtft/flexfb.c |7 +++ 2 files changed, 14 insertions(+) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 3638554..bd71487 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display, if (par-spi display-buswidth == 9) { par-spi-bits_per_word = 9; ret = spi_setup(par-spi); + if (!ret) { + struct spi_master *ma = par-spi-master; + + if (!(ma-bits_per_word_mask SPI_BPW_MASK(9))) + ret = -EINVAL; + } + if (ret) { dev_warn(par-spi-dev, 9-bit SPI not available, emulating using 8-bit.\n); diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c index 09fd15d..a54a8d9 100644 --- a/drivers/staging/fbtft/flexfb.c +++ b/drivers/staging/fbtft/flexfb.c @@ -465,6 +465,13 @@ static int flexfb_probe_common(struct spi_device *sdev, par-fbtftops.write_vmem = fbtft_write_vmem16_bus9; sdev-bits_per_word = 9; ret = spi_setup(sdev); + if (!ret) { + struct spi_master *ma = par-spi-master; + + if (!(ma-bits_per_word_mask SPI_BPW_MASK(9))) + ret = -EINVAL; + } + if (ret) { dev_warn(dev, 9-bit SPI not available, emulating using 8-bit.\n); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 1/2] staging: fbtft: replace master-setup() with spi_setup()
Calling the setup of the SPI master directly causes a NULL pointer dereference with master drivers without a separate setup function. This problem is reproduceable on ARM MXS platform. So fix this issue by using spi_setup() instead. Signed-off-by: Stefan Wahren stefan.wah...@i2se.com --- drivers/staging/fbtft/fb_uc1611.c|2 +- drivers/staging/fbtft/fb_watterott.c |4 ++-- drivers/staging/fbtft/fbtft-core.c |4 ++-- drivers/staging/fbtft/flexfb.c |4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/fbtft/fb_uc1611.c b/drivers/staging/fbtft/fb_uc1611.c index 32f3a9d..5cafa50 100644 --- a/drivers/staging/fbtft/fb_uc1611.c +++ b/drivers/staging/fbtft/fb_uc1611.c @@ -76,7 +76,7 @@ static int init_display(struct fbtft_par *par) /* Set CS active high */ par-spi-mode |= SPI_CS_HIGH; - ret = par-spi-master-setup(par-spi); + ret = spi_setup(par-spi); if (ret) { dev_err(par-info-device, Could not set SPI_CS_HIGH\n); return ret; diff --git a/drivers/staging/fbtft/fb_watterott.c b/drivers/staging/fbtft/fb_watterott.c index 88fb2c0..8eae6ef 100644 --- a/drivers/staging/fbtft/fb_watterott.c +++ b/drivers/staging/fbtft/fb_watterott.c @@ -169,7 +169,7 @@ static int init_display(struct fbtft_par *par) /* enable SPI interface by having CS and MOSI low during reset */ save_mode = par-spi-mode; par-spi-mode |= SPI_CS_HIGH; - ret = par-spi-master-setup(par-spi); /* set CS inactive low */ + ret = spi_setup(par-spi); /* set CS inactive low */ if (ret) { dev_err(par-info-device, Could not set SPI_CS_HIGH\n); return ret; @@ -180,7 +180,7 @@ static int init_display(struct fbtft_par *par) par-fbtftops.reset(par); mdelay(1000); par-spi-mode = save_mode; - ret = par-spi-master-setup(par-spi); + ret = spi_setup(par-spi); if (ret) { dev_err(par-info-device, Could not restore SPI mode\n); return ret; diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 23392eb..3638554 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -1437,12 +1437,12 @@ int fbtft_probe_common(struct fbtft_display *display, /* 9-bit SPI setup */ if (par-spi display-buswidth == 9) { par-spi-bits_per_word = 9; - ret = par-spi-master-setup(par-spi); + ret = spi_setup(par-spi); if (ret) { dev_warn(par-spi-dev, 9-bit SPI not available, emulating using 8-bit.\n); par-spi-bits_per_word = 8; - ret = par-spi-master-setup(par-spi); + ret = spi_setup(par-spi); if (ret) goto out_release; /* allocate buffer with room for dc bits */ diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c index c763efc..09fd15d 100644 --- a/drivers/staging/fbtft/flexfb.c +++ b/drivers/staging/fbtft/flexfb.c @@ -464,12 +464,12 @@ static int flexfb_probe_common(struct spi_device *sdev, par-fbtftops.write_register = fbtft_write_reg8_bus9; par-fbtftops.write_vmem = fbtft_write_vmem16_bus9; sdev-bits_per_word = 9; - ret = sdev-master-setup(sdev); + ret = spi_setup(sdev); if (ret) { dev_warn(dev, 9-bit SPI not available, emulating using 8-bit.\n); sdev-bits_per_word = 8; - ret = sdev-master-setup(sdev); + ret = spi_setup(sdev); if (ret) goto out_release; /* allocate buffer with room for dc bits */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 0/2] staging: fbtft: fix 9-bit SPI support
This patch series fixes the 9-bit SPI support of fbtft. Stefan Wahren (2): staging: fbtft: replace master-setup() with spi_setup() staging: fbtft: fix 9-bit SPI support detection drivers/staging/fbtft/fb_uc1611.c|2 +- drivers/staging/fbtft/fb_watterott.c |4 ++-- drivers/staging/fbtft/fbtft-core.c | 11 +-- drivers/staging/fbtft/flexfb.c | 11 +-- 4 files changed, 21 insertions(+), 7 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 0/9] Add simple NVMEM Framework via regmap.
Hi Srinivas, Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 24. Juni 2015 um 20:50 geschrieben: On 24/06/15 18:47, Stefan Wahren wrote: Hi Srinivas, Srinivas Kandagatla srinivas.kandaga...@linaro.org hat am 24. Juni 2015 um 15:03 geschrieben: On 24/06/15 13:30, Stefan Wahren wrote: If the question is just about hexdump, then hexdump itself can read file from given offset and size. yes, this is my question at first. Let me show the difference between the current implementation and my expectations as a user. $ hexdump /sys/class/nvmem/mxs-ocotp/nvmem Current implementation: dump the complete register range defined in DT Its dumping the range which is specified in the provider regmap. If the requirement is to dump only particular range, this has to be made explicit while creating regmap, which is to specify the base address to start from First data register and max_register to be Last data register - First data register i know about max_register, but i can't find the base address in regmap_config. Base is not in the regmap config, its the value which you pass to the thanks for you explanation. I was confused by the word base because in your example it is the context variable. Now i've successful tested my first version of the OCOTP driver based on NVMEM framework [1]. Currently only the raw sysfs access is working. So i have another question: How do i get the cell info from the devicetree for the nvmem_config? I expected a function like of_nvmem_cell_info_get() . Regards Stefan [1] - https://github.com/lategoodbye/fsl_ocotp/commit/7c98e19755b69f761885b0e1ceb2c258a7c47ade -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/