* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Zhi Li @ 2016-11-08 19:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAP=VYLo_p3ZmOFcyM9om_31-=5O3ZiRLCBkJDDtj84JxAA7h8Q@mail.gmail.com>
On Tue, Nov 8, 2016 at 1:00 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> On Mon, Sep 19, 2016 at 1:47 PM, Frank Li <Frank.Li@nxp.com> wrote:
>> From: Zhengyu Shen <zhengyu.shen@nxp.com>
>>
>> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
>> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
>> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
>> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
>> MMDC provides registers for performance counters which read via this
>> driver to help debug memory throughput and similar issues.
>>
>> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
>> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
>>
>> 898021787 mmdc/busy-cycles/
>> 14819600 mmdc/read-accesses/
>> 471.30 MB mmdc/read-bytes/
>> 2815419216 mmdc/total-cycles/
>> 13367354 mmdc/write-accesses/
>> 427.76 MB mmdc/write-bytes/
>>
>> 5.334757334 seconds time elapsed
>>
>> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
>> Signed-off-by: Frank Li <frank.li@nxp.com>
>> ---
>> Changes from v6 to v7
>> use mmdc_pmu prefix
>> remove unnecessary check
>> improve group event check according to mark's feedback.
>> check pmu_mmdc->mmdc_events[cfg] at event_add
>> only check == 0 at event_del
>>
>> Changes from v5 to v6
>> Improve group event error handle
>>
>> Changes from v4 to v5
>> Remove mmdc_pmu:irq
>> remove static variable cpuhp_mmdc_pmu
>> remove spin_lock
>> check is_sampling_event(event)
>> remove unnecessary cast
>> use hw_perf_event::prev_count
>>
>> Changes from v3 to v4:
>> Tested and fixed crash relating to removing events with perf fuzzer
>> Adjusted formatting
>> Moved all perf event code under CONFIG_PERF_EVENTS
>> Switched cpuhp_setup_state to cpuhp_setup_state_nocalls
>>
>> Changes from v2 to v3:
>> Use WARN_ONCE instead of returning generic error values
>> Replace CPU Notifiers with newer state machine hotplug
>> Added additional checks on event_init for grouping and sampling
>> Remove useless mmdc_enable_profiling function
>> Added comments
>> Moved start index of events from 0x01 to 0x00
>> Added a counter to pmu_mmdc to only stop hrtimer after all events are finished
>> Replace readl_relaxed and writel_relaxed with readl and writel
>> Removed duplicate update function
>> Used devm_kasprintf when naming mmdcs probed
>>
>> Changes from v1 to v2:
>> Added cpumask and migration handling support to driver
>> Validated event during event_init
>> Added code to properly stop counters
>> Used perf_invalid_context instead of perf_sw_context
>> Added hrtimer to poll for overflow
>> Added better description
>> Added support for multiple mmdcs
>>
>> arch/arm/mach-imx/mmdc.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 457 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
>> index db9621c..1f70376 100644
>> --- a/arch/arm/mach-imx/mmdc.c
>> +++ b/arch/arm/mach-imx/mmdc.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright 2011 Freescale Semiconductor, Inc.
>> + * Copyright 2011,2016 Freescale Semiconductor, Inc.
>> * Copyright 2011 Linaro Ltd.
>> *
>> * The code contained herein is licensed under the GNU General Public
>> @@ -10,12 +10,16 @@
>> * http://www.gnu.org/copyleft/gpl.html
>> */
>>
>> +#include <linux/hrtimer.h>
>> #include <linux/init.h>
>> +#include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/of_device.h>
>> +#include <linux/perf_event.h>
>> +#include <linux/slab.h>
>>
>> #include "common.h"
>>
>> @@ -27,8 +31,458 @@
>> #define BM_MMDC_MDMISC_DDR_TYPE 0x18
>> #define BP_MMDC_MDMISC_DDR_TYPE 0x3
>>
>> +#define TOTAL_CYCLES 0x0
>> +#define BUSY_CYCLES 0x1
>> +#define READ_ACCESSES 0x2
>> +#define WRITE_ACCESSES 0x3
>> +#define READ_BYTES 0x4
>> +#define WRITE_BYTES 0x5
>> +
>> +/* Enables, resets, freezes, overflow profiling*/
>> +#define DBG_DIS 0x0
>> +#define DBG_EN 0x1
>> +#define DBG_RST 0x2
>> +#define PRF_FRZ 0x4
>> +#define CYC_OVF 0x8
>> +
>> +#define MMDC_MADPCR0 0x410
>> +#define MMDC_MADPSR0 0x418
>> +#define MMDC_MADPSR1 0x41C
>> +#define MMDC_MADPSR2 0x420
>> +#define MMDC_MADPSR3 0x424
>> +#define MMDC_MADPSR4 0x428
>> +#define MMDC_MADPSR5 0x42C
>> +
>> +#define MMDC_NUM_COUNTERS 6
>> +
>> +#define to_mmdc_pmu(p) container_of(p, struct mmdc_pmu, pmu)
>> +
>> static int ddr_type;
>>
>> +#ifdef CONFIG_PERF_EVENTS
>> +
>> +static DEFINE_IDA(mmdc_ida);
>> +
>> +PMU_EVENT_ATTR_STRING(total-cycles, mmdc_pmu_total_cycles, "event=0x00")
>> +PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_pmu_busy_cycles, "event=0x01")
>> +PMU_EVENT_ATTR_STRING(read-accesses, mmdc_pmu_read_accesses, "event=0x02")
>> +PMU_EVENT_ATTR_STRING(write-accesses, mmdc_pmu_write_accesses, "config=0x03")
>> +PMU_EVENT_ATTR_STRING(read-bytes, mmdc_pmu_read_bytes, "event=0x04")
>> +PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_pmu_read_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_pmu_read_bytes_scale, "0.000001");
>> +PMU_EVENT_ATTR_STRING(write-bytes, mmdc_pmu_write_bytes, "event=0x05")
>> +PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_pmu_write_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_pmu_write_bytes_scale, "0.000001");
>> +
>> +struct mmdc_pmu {
>> + struct pmu pmu;
>> + void __iomem *mmdc_base;
>> + cpumask_t cpu;
>> + struct hrtimer hrtimer;
>> + unsigned int active_events;
>> + struct device *dev;
>> + struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
>> + struct hlist_node node;
>> +};
>> +
>> +/*
>> + * Polling period is set to one second, overflow of total-cycles (the fastest
>> + * increasing counter) takes ten seconds so one second is safe
>> + */
>> +static unsigned int mmdc_pmu_poll_period_us = 1000000;
>> +
>> +module_param_named(pmu_pmu_poll_period_us, mmdc_pmu_poll_period_us, uint,
>> + S_IRUGO | S_IWUSR);
>
> I just noticed this commit now that linux-next is back after the week off.
>
> This should probably use core_param or setup_param since the Kconfig
> for this is bool and not tristate. Similarly, that means that the .remove
> function you've added is dead code -- unless you envision a case where
> the user needs to forcibly unbind the driver...
>
> Do you want to redo the existing commit or do you want me to submit
> a follow-up fixup patch?
I will do follow-up fixup patch.
I think pmu_pmu_poll_period_us should be removed because no benefit to
change it.
I can delete .remove
best regards
Frank Li
>
> Thanks
> Paul.
> --
>
>> +
>> +static ktime_t mmdc_pmu_timer_period(void)
>> +{
>> + return ns_to_ktime((u64)mmdc_pmu_poll_period_us * 1000);
>> +}
>> +
^ permalink raw reply
* [PATCH 2/2] ARM: dts: apq8064: add support to pm8821
From: Bjorn Andersson @ 2016-11-08 19:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622577-20699-2-git-send-email-srinivas.kandagatla@linaro.org>
On Tue 08 Nov 08:29 PST 2016, Srinivas Kandagatla wrote:
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 1dbe697..fde006c 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -627,6 +627,34 @@
> clock-names = "core";
> };
>
> + qcom,ssbi at c00000 {
No "qcom," in the node name.
> + compatible = "qcom,ssbi";
> + reg = <0x00c00000 0x1000>;
> + qcom,controller-type = "pmic-arbiter";
> +
> + pmicintc2: pmic at 1 {
I think we should follow Linus' lead and label this "pm8821".
> + compatible = "qcom,pm8821";
> + interrupt-parent = <&tlmm_pinmux>;
> + interrupts = <76 8>;
Please spell out IRQ_TYPE_LEVEL_LOW.
And interrupts-extended = <&tlmm_pinmux 76 IRQ_TYPE_LEVEL_LOW> combines
the two lines nicely.
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pm8821_mpps: mpps at 50 {
> +
Extra newline.
> + compatible = "qcom,pm8821-mpp", "qcom,ssbi-mpp";
> + reg = <0x50>;
> +
> + interrupts = <24 1>, <25 1>, <26 1>,
> + <27 1>;
I think these should be IRQ_TYPE_NONE per the discussion on how to share
interrupts between the gpio/mpp driver and other clients.
On the other hand, per the pm8821 driver we only support LEVEL_LOW
(high?).
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
> + };
> +
Regards,
Bjorn
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Will Deacon @ 2016-11-08 19:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5822214F.2070500@redhat.com>
On Tue, Nov 08, 2016 at 02:02:39PM -0500, Don Dutile wrote:
> On 11/08/2016 12:54 PM, Will Deacon wrote:
> >A first step would be making all this opt-in, and only supporting GICv3
> >ITS for now.
> You're trying to support a config that is < GICv3 and no ITS ? ...
> That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
> hook was created ... to enable devel/kick-the-tires.
Yup, that's exactly what I was envisaging. For systems that can't do
passthrough safely, we'll always have to throw a devel switch.
Will
^ permalink raw reply
* [PATCH 1/2] mfd: pm8921: add support to pm8821
From: Bjorn Andersson @ 2016-11-08 19:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478622577-20699-1-git-send-email-srinivas.kandagatla@linaro.org>
On Tue 08 Nov 08:29 PST 2016, Srinivas Kandagatla wrote:
> This patch adds support to PM8821 PMIC and interrupt support.
> PM8821 is companion device that supplements primary PMIC PM8921 IC.
>
Linus Walleij has a patch out for renaming a lot of things in this file,
so we should probably make sure that lands and then rebase this ontop.
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Tested this patch for MPP and IRQ functionality on IFC6410 and SD600 EVAL
> board with mpps PM8821 and PM8921.
>
> .../devicetree/bindings/mfd/qcom-pm8xxx.txt | 1 +
> drivers/mfd/pm8921-core.c | 368 +++++++++++++++++++--
> 2 files changed, 340 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> index 37a088f..8f1b4ec 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> @@ -11,6 +11,7 @@ voltages and other various functionality to Qualcomm SoCs.
> Definition: must be one of:
> "qcom,pm8058"
> "qcom,pm8921"
> + "qcom,pm8821"
>
> - #address-cells:
> Usage: required
> diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
> index 0e3a2ea..28c2470 100644
> --- a/drivers/mfd/pm8921-core.c
> +++ b/drivers/mfd/pm8921-core.c
> @@ -28,16 +28,26 @@
> #include <linux/mfd/core.h>
>
> #define SSBI_REG_ADDR_IRQ_BASE 0x1BB
> -
> -#define SSBI_REG_ADDR_IRQ_ROOT (SSBI_REG_ADDR_IRQ_BASE + 0)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS1 (SSBI_REG_ADDR_IRQ_BASE + 1)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS2 (SSBI_REG_ADDR_IRQ_BASE + 2)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS3 (SSBI_REG_ADDR_IRQ_BASE + 3)
> -#define SSBI_REG_ADDR_IRQ_M_STATUS4 (SSBI_REG_ADDR_IRQ_BASE + 4)
> -#define SSBI_REG_ADDR_IRQ_BLK_SEL (SSBI_REG_ADDR_IRQ_BASE + 5)
> -#define SSBI_REG_ADDR_IRQ_IT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 6)
> -#define SSBI_REG_ADDR_IRQ_CONFIG (SSBI_REG_ADDR_IRQ_BASE + 7)
> -#define SSBI_REG_ADDR_IRQ_RT_STATUS (SSBI_REG_ADDR_IRQ_BASE + 8)
Keep these (per argumentation that follows), but try to name them
appropriately.
> +#define SSBI_PM8821_REG_ADDR_IRQ_BASE 0x100
> +
> +#define SSBI_REG_ADDR_IRQ_ROOT (0)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS1 (1)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS2 (2)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS3 (3)
> +#define SSBI_REG_ADDR_IRQ_M_STATUS4 (4)
> +#define SSBI_REG_ADDR_IRQ_BLK_SEL (5)
> +#define SSBI_REG_ADDR_IRQ_IT_STATUS (6)
> +#define SSBI_REG_ADDR_IRQ_CONFIG (7)
> +#define SSBI_REG_ADDR_IRQ_RT_STATUS (8)
Unnecessary parenthesis.
> +
> +#define PM8821_TOTAL_IRQ_MASTERS 2
Unused.
> +#define PM8821_BLOCKS_PER_MASTER 7
> +#define PM8821_IRQ_MASTER1_SET 0x01
BIT(0), but I would prefer that you just inline this with a comment.
> +#define PM8821_IRQ_CLEAR_OFFSET 0x01
Rather than having a single define for this and add in the base and
block numbers I think you should split it into a master0 and master1
define. (And it's not a offset as much as a register)
> +#define PM8821_IRQ_RT_STATUS_OFFSET 0x0f
Dito
> +#define PM8821_IRQ_MASK_REG_OFFSET 0x08
Dito
> +#define SSBI_REG_ADDR_IRQ_MASTER0 0x30
> +#define SSBI_REG_ADDR_IRQ_MASTER1 0xb0
Fold these two into the registers above.
>
> #define PM_IRQF_LVL_SEL 0x01 /* level select */
> #define PM_IRQF_MASK_FE 0x02 /* mask falling edge */
> @@ -54,30 +64,41 @@
> #define REG_HWREV_2 0x0E8 /* PMIC4 revision 2 */
>
> #define PM8921_NR_IRQS 256
> +#define PM8821_NR_IRQS 112
>
> struct pm_irq_chip {
> struct regmap *regmap;
> spinlock_t pm_irq_lock;
> struct irq_domain *irqdomain;
> + unsigned int irq_reg_base;
> unsigned int num_irqs;
> unsigned int num_blocks;
> unsigned int num_masters;
> u8 config[0];
> };
>
> +struct pm8xxx_data {
> + int num_irqs;
> + unsigned int irq_reg_base;
As far as I can see this is always SSBI_PM8821_REG_ADDR_IRQ_BASE in the
8821 functions and SSBI_REG_ADDR_IRQ_BASE in the pm8xxx functions. If
you have disjunct code paths I think it's better to not obscure this
with a variable.
Try renaming the constants appropriately instead. This also has the
benefit of reducing the size of the patch slightly.
> + const struct irq_domain_ops *irq_domain_ops;
> + void (*irq_handler)(struct irq_desc *desc);
> +};
> +
> static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, unsigned int bp,
> unsigned int *ip)
> {
> int rc;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
> goto bail;
> }
>
> - rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
> + rc = regmap_read(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
> if (rc)
> pr_err("Failed Reading Status rc=%d\n", rc);
> bail:
> @@ -91,14 +112,16 @@ pm8xxx_config_irq(struct pm_irq_chip *chip, unsigned int bp, unsigned int cp)
> int rc;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
> goto bail;
> }
>
> cp |= PM_IRQF_WRITE;
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_CONFIG, cp);
> + rc = regmap_write(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_CONFIG, cp);
> if (rc)
> pr_err("Failed Configuring IRQ rc=%d\n", rc);
> bail:
> @@ -137,8 +160,8 @@ static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
> unsigned int blockbits;
> int block_number, i, ret = 0;
>
> - ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_M_STATUS1 + master,
> - &blockbits);
> + ret = regmap_read(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_M_STATUS1 + master, &blockbits);
> if (ret) {
> pr_err("Failed to read master %d ret=%d\n", master, ret);
> return ret;
> @@ -165,7 +188,8 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
>
> chained_irq_enter(irq_chip, desc);
>
> - ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
> + ret = regmap_read(chip->regmap,
> + chip->irq_reg_base + SSBI_REG_ADDR_IRQ_ROOT, &root);
> if (ret) {
> pr_err("Can't read root status ret=%d\n", ret);
> return;
> @@ -182,6 +206,122 @@ static void pm8xxx_irq_handler(struct irq_desc *desc)
> chained_irq_exit(irq_chip, desc);
> }
>
> +static int pm8821_read_master_irq(const struct pm_irq_chip *chip,
> + int m, unsigned int *master)
> +{
I think you should inline this, as you already have the calls unrolled
in pm8821_irq_handler().
> + unsigned int base;
> +
> + if (!m)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + return regmap_read(chip->regmap, base, master);
> +}
> +
> +static int pm8821_read_block_irq(struct pm_irq_chip *chip, int master,
> + u8 block, unsigned int *bits)
> +{
> + int rc;
> +
> + unsigned int base;
Odd empty line between rc and base. (And btw, sorting your local
variables in descending length make things pretty).
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
The reason why this is done under a lock in the other case is because
the status register is paged, so you shouldn't need it here.
With this updated I think you can favorably inline this into
pm8821_irq_block_handler().
> +
> + rc = regmap_read(chip->regmap, base + block, bits);
> + if (rc)
> + pr_err("Failed Reading Status rc=%d\n", rc);
> +
> + spin_unlock(&chip->pm_irq_lock);
> + return rc;
> +}
> +
> +static int pm8821_irq_block_handler(struct pm_irq_chip *chip,
> + int master_number, int block)
> +{
> + int pmirq, irq, i, ret;
> + unsigned int bits;
> +
> + ret = pm8821_read_block_irq(chip, master_number, block, &bits);
> + if (ret) {
> + pr_err("Failed reading %d block ret=%d", block, ret);
> + return ret;
> + }
> + if (!bits) {
> + pr_err("block bit set in master but no irqs: %d", block);
> + return 0;
> + }
> +
> + /* Convert block offset to global block number */
> + block += (master_number * PM8821_BLOCKS_PER_MASTER) - 1;
So this is block -= 1 for master 0 and block += 6 for master 1, is the
latter correct?
> +
> + /* Check IRQ bits */
> + for (i = 0; i < 8; i++) {
> + if (bits & BIT(i)) {
> + pmirq = block * 8 + i;
> + irq = irq_find_mapping(chip->irqdomain, pmirq);
> + generic_handle_irq(irq);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int pm8821_irq_read_master(struct pm_irq_chip *chip,
> + int master_number, u8 master_val)
This isn't so much a matter of "reading master X" as "handle master X".
Also, you don't care about the return value, so no need to return one...
> +{
> + int ret = 0;
> + int block;
> +
> + for (block = 1; block < 8; block++) {
> + if (master_val & BIT(block)) {
> + ret |= pm8821_irq_block_handler(chip,
> + master_number, block);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void pm8821_irq_handler(struct irq_desc *desc)
> +{
> + struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
> + struct irq_chip *irq_chip = irq_desc_get_chip(desc);
> + int ret;
> + unsigned int master;
> +
> + chained_irq_enter(irq_chip, desc);
> + /* check master 0 */
> + ret = pm8821_read_master_irq(chip, 0, &master);
> + if (ret) {
> + pr_err("Failed to re:Qad master 0 ret=%d\n", ret);
> + return;
> + }
> +
> + if (master & ~PM8821_IRQ_MASTER1_SET)
Rather than having a define for MASTER1_SET use BIT(0) here and write a
comment like:
"bits 1 through 7 marks the first 7 blocks"
> + pm8821_irq_read_master(chip, 0, master);
> +
and then
"bit 0 is set if second master contains any bits"
Or just skip this optimization and check the two masters unconditionally
in a loop.
> + /* check master 1 */
> + if (!(master & PM8821_IRQ_MASTER1_SET))
> + goto done;
> +
> + ret = pm8821_read_master_irq(chip, 1, &master);
> + if (ret) {
> + pr_err("Failed to read master 1 ret=%d\n", ret);
> + return;
> + }
> +
> + pm8821_irq_read_master(chip, 1, master);
> +
> +done:
> + chained_irq_exit(irq_chip, desc);
> +}
> +
> static void pm8xxx_irq_mask_ack(struct irq_data *d)
> {
> struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> @@ -254,13 +394,15 @@ static int pm8xxx_irq_get_irqchip_state(struct irq_data *d,
> irq_bit = pmirq % 8;
>
> spin_lock(&chip->pm_irq_lock);
> - rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
> + rc = regmap_write(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_BLK_SEL, block);
> if (rc) {
> pr_err("Failed Selecting Block %d rc=%d\n", block, rc);
> goto bail;
> }
>
> - rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
> + rc = regmap_read(chip->regmap, chip->irq_reg_base +
> + SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
> if (rc) {
> pr_err("Failed Reading Status rc=%d\n", rc);
> goto bail;
> @@ -299,6 +441,151 @@ static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
> .map = pm8xxx_irq_domain_map,
> };
>
> +static void pm8821_irq_mask_ack(struct irq_data *d)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + unsigned int base, pmirq = irqd_to_hwirq(d);
> + u8 block, master;
> + int irq_bit, rc;
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
You can deobfuscate this somewhat by instead of testing for !master
below you just do:
if (block < PM8821_BLOCKS_PER_MASTER) {
base =
} else {
base =
block -= PM8821_BLOCKS_PER_MASTER;
}
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
The irqchip code grabs a lock on the irq_desc, so this can't race with
unmask - and the regmap_update_bits() is internally protecting the
read/write cycle.
So you shouldn't need to lock around this section.
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_MASK_REG_OFFSET + block,
> + BIT(irq_bit), BIT(irq_bit));
> +
> + if (rc) {
> + pr_err("Failed to read/write mask IRQ:%d rc=%d\n", pmirq, rc);
> + goto fail;
> + }
> +
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_CLEAR_OFFSET + block,
> + BIT(irq_bit), BIT(irq_bit));
> +
> + if (rc) {
> + pr_err("Failed to read/write IT_CLEAR IRQ:%d rc=%d\n",
> + pmirq, rc);
> + }
> +
> +fail:
> + spin_unlock(&chip->pm_irq_lock);
> +}
> +
> +static void pm8821_irq_unmask(struct irq_data *d)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + unsigned int base, pmirq = irqd_to_hwirq(d);
> + int irq_bit, rc;
> + u8 block, master;
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
As mask().
> +
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock(&chip->pm_irq_lock);
As mask().
> +
> + rc = regmap_update_bits(chip->regmap,
> + base + PM8821_IRQ_MASK_REG_OFFSET + block,
> + BIT(irq_bit), ~BIT(irq_bit));
> +
> + if (rc)
> + pr_err("Failed to read/write unmask IRQ:%d rc=%d\n", pmirq, rc);
> +
> + spin_unlock(&chip->pm_irq_lock);
> +}
> +
> +static int pm8821_irq_set_type(struct irq_data *d, unsigned int flow_type)
> +{
> +
> + /*
> + * PM8821 IRQ controller does not have explicit software support for
> + * IRQ flow type.
> + */
Is returning "success" here the right thing to do? Shouldn't we just
omit the function? Or did you perhaps hit some clients that wouldn't
deal with that?
> + return 0;
> +}
> +
> +static int pm8821_irq_get_irqchip_state(struct irq_data *d,
> + enum irqchip_irq_state which,
> + bool *state)
> +{
> + struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
> + int pmirq, rc;
> + u8 block, irq_bit, master;
> + unsigned int bits;
> + unsigned int base;
> + unsigned long flags;
> +
> + pmirq = irqd_to_hwirq(d);
> +
> + block = pmirq / 8;
> + master = block / PM8821_BLOCKS_PER_MASTER;
> + irq_bit = pmirq % 8;
> + block %= PM8821_BLOCKS_PER_MASTER;
> +
Simplify as in mask().
> + if (!master)
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER0;
> + else
> + base = chip->irq_reg_base + SSBI_REG_ADDR_IRQ_MASTER1;
> +
> + spin_lock_irqsave(&chip->pm_irq_lock, flags);
No need to lock here as we're just reading a single register.
> +
> + rc = regmap_read(chip->regmap,
> + base + PM8821_IRQ_RT_STATUS_OFFSET + block, &bits);
> + if (rc) {
> + pr_err("Failed Reading Status rc=%d\n", rc);
> + goto bail_out;
> + }
> +
> + *state = !!(bits & BIT(irq_bit));
> +
> +bail_out:
> + spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
> +
> + return rc;
> +}
> +
> +static struct irq_chip pm8821_irq_chip = {
> + .name = "pm8821",
> + .irq_mask_ack = pm8821_irq_mask_ack,
> + .irq_unmask = pm8821_irq_unmask,
> + .irq_set_type = pm8821_irq_set_type,
> + .irq_get_irqchip_state = pm8821_irq_get_irqchip_state,
> + .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_SKIP_SET_WAKE,
> +};
> +
Regards,
Bjorn
^ permalink raw reply
* [RESPIN-PATCH v2 1/2] ARM: EXYNOS: Remove static mapping of SCU SFR
From: Krzysztof Kozlowski @ 2016-11-08 19:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478603984-504-2-git-send-email-pankaj.dubey@samsung.com>
On Tue, Nov 08, 2016 at 04:49:43PM +0530, Pankaj Dubey wrote:
> Lets remove static mapping of SCU SFR mainly used in CORTEX-A9 SoC based boards.
> Instead use mapping from device tree node of SCU.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
> arch/arm/mach-exynos/common.h | 1 +
> arch/arm/mach-exynos/exynos.c | 22 ------------------
> arch/arm/mach-exynos/include/mach/map.h | 2 --
> arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++++-------
> arch/arm/mach-exynos/pm.c | 5 ++--
> arch/arm/mach-exynos/suspend.c | 13 ++++-------
> arch/arm/plat-samsung/include/plat/map-s5p.h | 4 ----
> 7 files changed, 34 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 9424a8a..dd5d8e8 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -161,6 +161,7 @@ extern void exynos_cpu_restore_register(void);
> extern void exynos_pm_central_suspend(void);
> extern int exynos_pm_central_resume(void);
> extern void exynos_enter_aftr(void);
> +extern int exynos_scu_enable(void);
>
> extern struct cpuidle_exynos_data cpuidle_coupled_exynos_data;
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 757fc11..fa08ef9 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -28,15 +28,6 @@
>
> #include "common.h"
>
> -static struct map_desc exynos4_iodesc[] __initdata = {
> - {
> - .virtual = (unsigned long)S5P_VA_COREPERI_BASE,
> - .pfn = __phys_to_pfn(EXYNOS4_PA_COREPERI),
> - .length = SZ_8K,
> - .type = MT_DEVICE,
> - },
> -};
> -
> static struct platform_device exynos_cpuidle = {
> .name = "exynos_cpuidle",
> #ifdef CONFIG_ARM_EXYNOS_CPUIDLE
> @@ -99,17 +90,6 @@ static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
> return 1;
> }
>
> -/*
> - * exynos_map_io
> - *
> - * register the standard cpu IO areas
> - */
> -static void __init exynos_map_io(void)
> -{
> - if (soc_is_exynos4())
> - iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> -}
> -
> static void __init exynos_init_io(void)
> {
> debug_ll_io_init();
> @@ -118,8 +98,6 @@ static void __init exynos_init_io(void)
>
> /* detect cpu id and rev. */
> s5p_init_cpu(S5P_VA_CHIPID);
> -
> - exynos_map_io();
> }
>
> /*
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 5fb0040..0eef407 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -18,6 +18,4 @@
>
> #define EXYNOS_PA_CHIPID 0x10000000
>
> -#define EXYNOS4_PA_COREPERI 0x10500000
> -
> #endif /* __ASM_ARCH_MAP_H */
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index a5d6841..94405c7 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -168,6 +168,27 @@ int exynos_cluster_power_state(int cluster)
> S5P_CORE_LOCAL_PWR_EN);
> }
>
> +/**
> + * exynos_scu_enable : enables SCU for Cortex-A9 based system
> + * returns 0 on success else non-zero error code
> + */
> +int exynos_scu_enable(void)
> +{
> + struct device_node *np;
> + void __iomem *scu_base;
> +
> + np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> + scu_base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!scu_base) {
> + pr_err("%s failed to map scu_base\n", __func__);
> + return -ENOMEM;
> + }
> + scu_enable(scu_base);
> + iounmap(scu_base);
> + return 0;
> +}
> +
> static void __iomem *cpu_boot_reg_base(void)
> {
> if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
> @@ -224,11 +245,6 @@ static void write_pen_release(int val)
> sync_cache_w(&pen_release);
> }
>
> -static void __iomem *scu_base_addr(void)
> -{
> - return (void __iomem *)(S5P_VA_SCU);
> -}
> -
> static DEFINE_SPINLOCK(boot_lock);
>
> static void exynos_secondary_init(unsigned int cpu)
> @@ -393,9 +409,11 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
>
> exynos_set_delayed_reset_assertion(true);
>
> - if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9)
> - scu_enable(scu_base_addr());
> -
> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> + /* if exynos_scu_enable fails, return */
> + if (exynos_scu_enable())
> + return;
Ohhh, someone (e.g. out-of-tree DTS) might be surprised with this.
Please mention such change of behaviour in the commit log (describe the
possible impact of this commit).
> + }
> /*
> * Write the address of secondary startup into the
> * system-wide flags register. The boot monitor waits
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 487295f..23db2af 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -18,6 +18,7 @@
> #include <linux/cpu_pm.h>
> #include <linux/io.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
Why do you need this include? Was it coming from mach/map.h?
> #include <linux/soc/samsung/exynos-regs-pmu.h>
> #include <linux/soc/samsung/exynos-pmu.h>
>
> @@ -26,8 +27,6 @@
> #include <asm/suspend.h>
> #include <asm/cacheflush.h>
>
> -#include <mach/map.h>
> -
> #include "common.h"
>
> static inline void __iomem *exynos_boot_vector_addr(void)
> @@ -177,7 +176,7 @@ void exynos_enter_aftr(void)
> cpu_suspend(0, exynos_aftr_finisher);
>
> if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> - scu_enable(S5P_VA_SCU);
> + exynos_scu_enable();
> if (call_firmware_op(resume) == -ENOSYS)
> exynos_cpu_restore_register();
> }
> diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
> index 06332f6..c73c857 100644
> --- a/arch/arm/mach-exynos/suspend.c
> +++ b/arch/arm/mach-exynos/suspend.c
> @@ -34,8 +34,6 @@
> #include <asm/smp_scu.h>
> #include <asm/suspend.h>
>
> -#include <mach/map.h>
> -
> #include <plat/pm-common.h>
>
> #include "common.h"
> @@ -461,12 +459,11 @@ static void exynos_pm_resume(void)
> /* For release retention */
> exynos_pm_release_retention();
>
> - if (cpuid == ARM_CPU_PART_CORTEX_A9)
> - scu_enable(S5P_VA_SCU);
> -
> - if (call_firmware_op(resume) == -ENOSYS
> - && cpuid == ARM_CPU_PART_CORTEX_A9)
> - exynos_cpu_restore_register();
> + if (cpuid == ARM_CPU_PART_CORTEX_A9) {
> + exynos_scu_enable();
> + if (call_firmware_op(resume) == -ENOSYS)
> + exynos_cpu_restore_register();
It does not look right. I think you changed the logic here. Previously
if CPU != A9, then call_firmware_op() was executed. Now it won't be.
BTW, don't respin patchset with the same version number. This is
basically v3. To me, increasing version number is always welcomed. It
makes dealign with patches easier.
Best regards,
Krzysztof
> + }
>
> early_wakeup:
>
> diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
> index 0fe2828..512ed1f 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s5p.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
> @@ -15,10 +15,6 @@
>
> #define S5P_VA_CHIPID S3C_ADDR(0x02000000)
>
> -#define S5P_VA_COREPERI_BASE S3C_ADDR(0x02800000)
> -#define S5P_VA_COREPERI(x) (S5P_VA_COREPERI_BASE + (x))
> -#define S5P_VA_SCU S5P_VA_COREPERI(0x0)
> -
> #define VA_VIC(x) (S3C_VA_IRQ + ((x) * 0x10000))
> #define VA_VIC0 VA_VIC(0)
> #define VA_VIC1 VA_VIC(1)
> --
> 2.7.4
>
^ permalink raw reply
* KASAN & the vmalloc area
From: Mark Rutland @ 2016-11-08 19:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I see a while back [1] there was a discussion of what to do about KASAN
and vmapped stacks, but it doesn't look like that was solved, judging by
the vmapped stacks pull [2] for v4.9.
I wondered whether anyone had looked at that since?
I have an additional reason to want to dynamically allocate the vmalloc
area shadow: it turns out that KASAN currently interacts rather poorly
with the arm64 ptdump code.
When KASAN is selected, we allocate shadow for the whole vmalloc area,
using common zero pte, pmd, pud tables. Walking over these in the ptdump
code takes a *very* long time (I've seen up to 15 minutes with
KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
long, too.
If I don't allocate vmalloc shadow (and remove the apparently pointlesss
shadow of the shadow area), and only allocate shadow for the image,
fixmap, vmemmap and so on, that delay gets cut to a few seconds, which
is tolerable for a debug configuration...
... however, things blow up when the kernel touches vmalloc'd memory for
the first time, as we don't install shadow for that dynamically.
Thanks,
Mark.
[1] https://lkml.kernel.org/r/CALCETrWucrYp+yq8RHSDqf93xtg793duByirurzJbLRhrz=tcA at mail.gmail.com
[2] https://lkml.kernel.org/r/20161003092940.GA691 at gmail.com
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/464191.html
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Don Dutile @ 2016-11-08 19:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108175457.GK20591@arm.com>
On 11/08/2016 12:54 PM, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
>> On 08/11/2016 03:45, Will Deacon wrote:
>>> Rather than treat these as separate problems, a better interface is to
>>> tell userspace about a set of reserved regions, and have this include
>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>> Don suggested that we statically pick an address for the doorbell in a
>>> similar way to x86, and have the kernel map it there. We could even pick
>>> 0xfee00000. If it conflicts with a reserved region on the platform (due
>>> to (4)), then we'd obviously have to (deterministically?) allocate it
>>> somewhere else, but probably within the bottom 4G.
>> This is tentatively achieved now with
>> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
>> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1264506.html)
> Yup, I saw that fly by. Hopefully some of the internals can be reused
> with the current thinking on user ABI.
>
>>> The next question is how to tell userspace about all of the reserved
>>> regions. Initially, the idea was to extend VFIO, however Alex pointed
>>> out a horrible scenario:
>>>
>>> 1. QEMU spawns a VM on system 0
>>> 2. VM is migrated to system 1
>>> 3. QEMU attempts to passthrough a device using PCI hotplug
>>>
>>> In this scenario, the guest memory map is chosen at step (1), yet there
>>> is no VFIO fd available to determine the reserved regions. Furthermore,
>>> the reserved regions may vary between system 0 and system 1. This pretty
>>> much rules out using VFIO to determine the reserved regions.Alex suggested
>>> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
>>> would solve part of the problem, but migration between systems with
>>> different memory maps can still cause problems if the reserved regions
>>> of the new system conflict with the guest memory map chosen by QEMU.
>>
>> OK so I understand we do not want anymore the VFIO chain capability API
>> (patch 5 of above series) but we prefer a sysfs approach instead.
> Right.
>
>> I understand the sysfs approach which allows the userspace to get the
>> info earlier and independently on VFIO. Keeping in mind current QEMU
>> virt - which is not the only userspace - will not do much from this info
>> until we bring upheavals in virt address space management. So if I am
>> not wrong, at the moment the main action to be undertaken is the
>> rejection of the PCI hotplug in case we detect a collision.
> I don't think so; it should be up to userspace to reject the hotplug.
> If userspace doesn't have support for the regions, then that's fine --
> you just end up in a situation where the CPU page table maps memory
> somewhere that the device can't see. In other words, you'll end up with
> spurious DMA failures, but that's exactly what happens with current systems
> if you passthrough an overlapping region (Robin demonstrated this on Juno).
>
> Additionally, you can imagine some future support where you can tell the
> guest not to use certain regions of its memory for DMA. In this case, you
> wouldn't want to refuse the hotplug in the case of overlapping regions.
>
> Really, I think the kernel side just needs to enumerate the fixed reserved
> regions, place the doorbell at a fixed address and then advertise these
> via sysfs.
>
>> I can respin [1]
>> - studying and taking into account Robin's comments about dm_regions
>> similarities
>> - removing the VFIO capability chain and replacing this by a sysfs API
> Ideally, this would be reusable between different SMMU drivers so the sysfs
> entries have the same format etc.
>
>> Would that be OK?
> Sounds good to me. Are you in a position to prototype something on the qemu
> side once we've got kernel-side agreement?
>
>> What about Alex comments who wanted to report the usable memory ranges
>> instead of unusable memory ranges?
>>
>> Also did you have a chance to discuss the following items:
>> 1) the VFIO irq safety assessment
> The discussion really focussed on system topology, as opposed to properties
> of the doorbell. Regardless of how the device talks to the doorbell, if
> the doorbell can't protect against things like MSI spoofing, then it's
> unsafe. My opinion is that we shouldn't allow passthrough by default on
> systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
> cmdline option to VFIO).
>
> A first step would be making all this opt-in, and only supporting GICv3
> ITS for now.
You're trying to support a config that is < GICv3 and no ITS ? ...
That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
hook was created ... to enable devel/kick-the-tires.
>> 2) the MSI reserved size computation (is an arbitrary size OK?)
> If we fix the base address, we could fix a size too. However, we'd still
> need to enumerate the doorbells to check that they fit in the region we
> have. If not, then we can warn during boot and treat it the same way as
> a resource conflict (that is, reallocate the region in some deterministic
> way).
>
> Will
^ permalink raw reply
* [PATCH 1/1 v7] ARM: imx: Added perf functionality to mmdc driver
From: Paul Gortmaker @ 2016-11-08 19:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474307267-3081-1-git-send-email-Frank.Li@nxp.com>
On Mon, Sep 19, 2016 at 1:47 PM, Frank Li <Frank.Li@nxp.com> wrote:
> From: Zhengyu Shen <zhengyu.shen@nxp.com>
>
> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
> MMDC provides registers for performance counters which read via this
> driver to help debug memory throughput and similar issues.
>
> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
>
> 898021787 mmdc/busy-cycles/
> 14819600 mmdc/read-accesses/
> 471.30 MB mmdc/read-bytes/
> 2815419216 mmdc/total-cycles/
> 13367354 mmdc/write-accesses/
> 427.76 MB mmdc/write-bytes/
>
> 5.334757334 seconds time elapsed
>
> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
> Signed-off-by: Frank Li <frank.li@nxp.com>
> ---
> Changes from v6 to v7
> use mmdc_pmu prefix
> remove unnecessary check
> improve group event check according to mark's feedback.
> check pmu_mmdc->mmdc_events[cfg] at event_add
> only check == 0 at event_del
>
> Changes from v5 to v6
> Improve group event error handle
>
> Changes from v4 to v5
> Remove mmdc_pmu:irq
> remove static variable cpuhp_mmdc_pmu
> remove spin_lock
> check is_sampling_event(event)
> remove unnecessary cast
> use hw_perf_event::prev_count
>
> Changes from v3 to v4:
> Tested and fixed crash relating to removing events with perf fuzzer
> Adjusted formatting
> Moved all perf event code under CONFIG_PERF_EVENTS
> Switched cpuhp_setup_state to cpuhp_setup_state_nocalls
>
> Changes from v2 to v3:
> Use WARN_ONCE instead of returning generic error values
> Replace CPU Notifiers with newer state machine hotplug
> Added additional checks on event_init for grouping and sampling
> Remove useless mmdc_enable_profiling function
> Added comments
> Moved start index of events from 0x01 to 0x00
> Added a counter to pmu_mmdc to only stop hrtimer after all events are finished
> Replace readl_relaxed and writel_relaxed with readl and writel
> Removed duplicate update function
> Used devm_kasprintf when naming mmdcs probed
>
> Changes from v1 to v2:
> Added cpumask and migration handling support to driver
> Validated event during event_init
> Added code to properly stop counters
> Used perf_invalid_context instead of perf_sw_context
> Added hrtimer to poll for overflow
> Added better description
> Added support for multiple mmdcs
>
> arch/arm/mach-imx/mmdc.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 457 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
> index db9621c..1f70376 100644
> --- a/arch/arm/mach-imx/mmdc.c
> +++ b/arch/arm/mach-imx/mmdc.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011,2016 Freescale Semiconductor, Inc.
> * Copyright 2011 Linaro Ltd.
> *
> * The code contained herein is licensed under the GNU General Public
> @@ -10,12 +10,16 @@
> * http://www.gnu.org/copyleft/gpl.html
> */
>
> +#include <linux/hrtimer.h>
> #include <linux/init.h>
> +#include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> +#include <linux/perf_event.h>
> +#include <linux/slab.h>
>
> #include "common.h"
>
> @@ -27,8 +31,458 @@
> #define BM_MMDC_MDMISC_DDR_TYPE 0x18
> #define BP_MMDC_MDMISC_DDR_TYPE 0x3
>
> +#define TOTAL_CYCLES 0x0
> +#define BUSY_CYCLES 0x1
> +#define READ_ACCESSES 0x2
> +#define WRITE_ACCESSES 0x3
> +#define READ_BYTES 0x4
> +#define WRITE_BYTES 0x5
> +
> +/* Enables, resets, freezes, overflow profiling*/
> +#define DBG_DIS 0x0
> +#define DBG_EN 0x1
> +#define DBG_RST 0x2
> +#define PRF_FRZ 0x4
> +#define CYC_OVF 0x8
> +
> +#define MMDC_MADPCR0 0x410
> +#define MMDC_MADPSR0 0x418
> +#define MMDC_MADPSR1 0x41C
> +#define MMDC_MADPSR2 0x420
> +#define MMDC_MADPSR3 0x424
> +#define MMDC_MADPSR4 0x428
> +#define MMDC_MADPSR5 0x42C
> +
> +#define MMDC_NUM_COUNTERS 6
> +
> +#define to_mmdc_pmu(p) container_of(p, struct mmdc_pmu, pmu)
> +
> static int ddr_type;
>
> +#ifdef CONFIG_PERF_EVENTS
> +
> +static DEFINE_IDA(mmdc_ida);
> +
> +PMU_EVENT_ATTR_STRING(total-cycles, mmdc_pmu_total_cycles, "event=0x00")
> +PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_pmu_busy_cycles, "event=0x01")
> +PMU_EVENT_ATTR_STRING(read-accesses, mmdc_pmu_read_accesses, "event=0x02")
> +PMU_EVENT_ATTR_STRING(write-accesses, mmdc_pmu_write_accesses, "config=0x03")
> +PMU_EVENT_ATTR_STRING(read-bytes, mmdc_pmu_read_bytes, "event=0x04")
> +PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_pmu_read_bytes_unit, "MB");
> +PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_pmu_read_bytes_scale, "0.000001");
> +PMU_EVENT_ATTR_STRING(write-bytes, mmdc_pmu_write_bytes, "event=0x05")
> +PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_pmu_write_bytes_unit, "MB");
> +PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_pmu_write_bytes_scale, "0.000001");
> +
> +struct mmdc_pmu {
> + struct pmu pmu;
> + void __iomem *mmdc_base;
> + cpumask_t cpu;
> + struct hrtimer hrtimer;
> + unsigned int active_events;
> + struct device *dev;
> + struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
> + struct hlist_node node;
> +};
> +
> +/*
> + * Polling period is set to one second, overflow of total-cycles (the fastest
> + * increasing counter) takes ten seconds so one second is safe
> + */
> +static unsigned int mmdc_pmu_poll_period_us = 1000000;
> +
> +module_param_named(pmu_pmu_poll_period_us, mmdc_pmu_poll_period_us, uint,
> + S_IRUGO | S_IWUSR);
I just noticed this commit now that linux-next is back after the week off.
This should probably use core_param or setup_param since the Kconfig
for this is bool and not tristate. Similarly, that means that the .remove
function you've added is dead code -- unless you envision a case where
the user needs to forcibly unbind the driver...
Do you want to redo the existing commit or do you want me to submit
a follow-up fixup patch?
Thanks
Paul.
--
> +
> +static ktime_t mmdc_pmu_timer_period(void)
> +{
> + return ns_to_ktime((u64)mmdc_pmu_poll_period_us * 1000);
> +}
> +
^ permalink raw reply
* [PATCH 3/4] fpga mgr: zynq: Add support for encrypted bitstreams
From: Moritz Fischer @ 2016-11-08 18:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108183217.GV14444@xsjsorenbubuntu>
Hi S?ren,
On Tue, Nov 8, 2016 at 10:32 AM, S?ren Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Sun, 2016-11-06 at 17:13:25 -0700, Moritz Fischer wrote:
>> Add new flag FPGA_MGR_DECRYPT_BISTREAM as well as a matching
>> capability FPGA_MGR_CAP_DECRYPT to allow for on-the-fly
>> decryption of an encrypted bitstream.
>>
>> If the system is not booted in secure mode AES & HMAC units
>> are disabled by the boot ROM, therefore the capability
>> is not available.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> Cc: Alan Tull <atull@opensource.altera.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: linux-kernel at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org
>> ---
>> drivers/fpga/fpga-mgr.c | 7 +++++++
>> drivers/fpga/zynq-fpga.c | 21 +++++++++++++++++++--
>> include/linux/fpga/fpga-mgr.h | 2 ++
>> 3 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
>> index 98230b7..e4d08e1 100644
>> --- a/drivers/fpga/fpga-mgr.c
>> +++ b/drivers/fpga/fpga-mgr.c
>> @@ -61,6 +61,12 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>> return -ENOTSUPP;
>> }
>>
>> + if (flags & FPGA_MGR_DECRYPT_BITSTREAM &&
>> + !fpga_mgr_has_cap(FPGA_MGR_CAP_DECRYPT, mgr->caps)) {
>> + dev_err(dev, "Bitstream decryption not supported\n");
>> + return -ENOTSUPP;
>> + }
>> +
>> /*
>> * Call the low level driver's write_init function. This will do the
>> * device-specific things to get the FPGA into the state where it is
>> @@ -170,6 +176,7 @@ static const char * const state_str[] = {
>> static const char * const cap_str[] = {
>> [FPGA_MGR_CAP_FULL_RECONF] = "Full reconfiguration",
>> [FPGA_MGR_CAP_PARTIAL_RECONF] = "Partial reconfiguration",
>> + [FPGA_MGR_CAP_DECRYPT] = "Decrypt bitstream on the fly",
>> };
>>
>> static ssize_t name_show(struct device *dev,
>> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
>> index 1d37ff0..0aa4705 100644
>> --- a/drivers/fpga/zynq-fpga.c
>> +++ b/drivers/fpga/zynq-fpga.c
>> @@ -71,6 +71,10 @@
>> #define CTRL_PCAP_PR_MASK BIT(27)
>> /* Enable PCAP */
>> #define CTRL_PCAP_MODE_MASK BIT(26)
>> +/* Needed to reduce clock rate for secure config */
>> +#define CTRL_PCAP_RATE_EN_MASK BIT(25)
>> +/* System booted in secure mode */
>> +#define CTRL_SEC_EN_MASK BIT(7)
>>
>> /* Miscellaneous Control Register bit definitions */
>> /* Internal PCAP loopback */
>> @@ -252,12 +256,20 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>
>> /* set configuration register with following options:
>> * - enable PCAP interface
>> - * - set throughput for maximum speed
>> + * - set throughput for maximum speed (if we're not decrypting)
>> * - set CPU in user mode
>> */
>> ctrl = zynq_fpga_read(priv, CTRL_OFFSET);
>> - zynq_fpga_write(priv, CTRL_OFFSET,
>> + if (flags & FPGA_MGR_DECRYPT_BITSTREAM) {
>> + zynq_fpga_write(priv, CTRL_OFFSET,
>> + (CTRL_PCAP_PR_MASK | CTRL_PCAP_MODE_MASK |
>> + CTRL_PCAP_RATE_EN_MASK | ctrl));
>> +
>> + } else {
>> + ctrl &= ~CTRL_PCAP_RATE_EN_MASK;
>> + zynq_fpga_write(priv, CTRL_OFFSET,
>> (CTRL_PCAP_PR_MASK | CTRL_PCAP_MODE_MASK | ctrl));
>> + }
>
> Minor nit:
> Assuming that there may be more caps to check to come, wouldn't it be
> slightly easier to write this in a way like?:
> if (flags & SOME_FLAG)
> ctrl |= FOO;
> if (flags & SOME_OTHER_FLAG)
> ctrl |= BAR;
> zynq_fpga_write(priv, CTRL_OFFSET, ctrl);
>
> i.e. moving the fpga_write outside of the conditionals.
Yeah, will do. Definitely better that way.
Thanks for the review,
Moritz
^ permalink raw reply
* [PATCH 3/3] ARM: dts: da850: add usb device node
From: Axel Haslam @ 2016-11-08 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185831.17683-1-ahaslam@baylibre.com>
This adds the ohci device node for the da850 soc.
It also enables it for the omapl138 hawk board.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
arch/arm/boot/dts/da850.dtsi | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..aaf533e 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -86,6 +86,14 @@
};
};
+&usb_phy {
+ status = "okay";
+};
+
+&ohci {
+ status = "okay";
+};
+
&serial2 {
pinctrl-names = "default";
pinctrl-0 = <&serial2_rxtx_pins>;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 2534aab..c74f1a6 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -405,6 +405,14 @@
>;
status = "disabled";
};
+ ohci: usb at 225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+ };
gpio: gpio at 226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
--
2.10.1
^ permalink raw reply related
* [PATCH 2/3] USB: ohci: da8xx: Allow probing from DT
From: Axel Haslam @ 2016-11-08 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185831.17683-1-ahaslam@baylibre.com>
This adds the compatible string to the ohci driver
to be able to probe from DT
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 83b182e..bbfe342 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -321,6 +321,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
/*-------------------------------------------------------------------------*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+ { .compatible = "ti,da830-ohci" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+#endif
static int ohci_da8xx_probe(struct platform_device *pdev)
{
@@ -472,6 +479,7 @@ static struct platform_driver ohci_hcd_da8xx_driver = {
#endif
.driver = {
.name = DRV_NAME,
+ .of_match_table = of_match_ptr(da8xx_ohci_ids),
},
};
--
2.10.1
^ permalink raw reply related
* [PATCH 1/3] USB: ohci: da8xx: Add devicetree bindings
From: Axel Haslam @ 2016-11-08 18:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185831.17683-1-ahaslam@baylibre.com>
This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's
Cc: devicetree at vger.kernel.org
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
.../devicetree/bindings/usb/ohci-da8xx.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 0000000..66672e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,39 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg: Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys: Phandle for the PHY device
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: Regulator that controls vbus power
+
+Example:
+
+reg_usb_ohci: regulator at 0 {
+ compatible = "regulator-fixed";
+ gpio = <&gpio 109 0>;
+ over-current-gpios = <&gpio 36 0>;
+ regulator-boot-on;
+ enable-active-high;
+ regulator-name = "usb_ohci_vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+};
+
+usb_phy: usb-phy {
+ compatible = "ti,da830-usb-phy";
+ #phy-cells = <1>;
+};
+
+ohci: usb at 0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ vbus-supply = <®_usb_ohci>;
+};
--
2.10.1
^ permalink raw reply related
* [PATCH 0/3] [PART 4/4] USB: ohci-da8xx: Add DT support
From: Axel Haslam @ 2016-11-08 18:58 UTC (permalink / raw)
To: linux-arm-kernel
This series defines the bindings and adds device tree support
for the davinci OHCI driver.
DEPENDENCIES:
1. [PATCH 0/3] fix ohci phy name
https://lkml.org/lkml/2016/11/2/208
2. [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers
https://lkml.org/lkml/2016/11/3/188
3. [PATCH v4 0/3] [PART 1/4] USB: ohci-da8xx: Allow a regulator for VBUS and over current
4. [PATCH 0/3] [PART 2/4] ARM: davinci: OHCI: Use a regulator instead of callbacks
5. [PATCH 0/2] [PART 3/4] USB: ohci-da8xx: Remove platform callbacks
A branch with all the dependencies can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt-v4
Axel Haslam (3):
USB: ohci: da8xx: Add devicetree bindings
USB: ohci: da8xx: Allow probing from DT
ARM: dts: da850: add usb device node
.../devicetree/bindings/usb/ohci-da8xx.txt | 39 ++++++++++++++++++++++
arch/arm/boot/dts/da850-lcdk.dts | 8 +++++
arch/arm/boot/dts/da850.dtsi | 8 +++++
drivers/usb/host/ohci-da8xx.c | 8 +++++
4 files changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
--
2.10.1
^ permalink raw reply
* [PATCH 2/2] USB: ohci: da8xx: use a flag instead of mask for ocic
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185752.17621-1-ahaslam@baylibre.com>
Now that the platform callback is removed, we can move the over
current indictor changed flag to the private data structure.
Since the driver only handles a single port, there is no need
for ocic to be a mask, we can use a simple flag instead.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 3dcbf1f..83b182e 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -43,12 +43,10 @@ struct da8xx_ohci_hcd {
struct regulator *vbus_reg;
struct notifier_block nb;
unsigned int is_powered;
+ unsigned int oc_changed;
};
#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
-/* Over-current indicator change bitmask */
-static volatile u16 ocic_mask;
-
static int ohci_da8xx_enable(struct usb_hcd *hcd)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
@@ -168,7 +166,7 @@ static int ohci_da8xx_regulator_event(struct notifier_block *nb,
if (event & REGULATOR_EVENT_OVER_CURRENT) {
dev_warn(dev, "over current event\n");
- ocic_mask |= 1;
+ da8xx_ohci->oc_changed = 1;
ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
}
@@ -241,10 +239,11 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
*/
static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int length = orig_ohci_hub_status_data(hcd, buf);
/* See if we have OCIC bit set on port 1 */
- if (ocic_mask & (1 << 1)) {
+ if (da8xx_ohci->oc_changed) {
dev_dbg(hcd->self.controller, "over-current indicator change "
"on port 1\n");
@@ -262,6 +261,7 @@ static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
int temp;
@@ -284,7 +284,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
- if (ocic_mask & (1 << wIndex))
+ if (da8xx_ohci->oc_changed)
temp |= RH_PS_OCIC;
put_unaligned(cpu_to_le32(temp), (__le32 *)buf);
@@ -311,10 +311,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp ? "Set" : "Clear", wIndex,
"C_OVER_CURRENT");
- if (temp)
- ocic_mask |= 1 << wIndex;
- else
- ocic_mask &= ~(1 << wIndex);
+ da8xx_ohci->oc_changed = temp;
return 0;
}
}
--
2.10.1
^ permalink raw reply related
* [PATCH 1/2] USB: ohci: da8xx: Remove ohci platform callbacks
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185752.17621-1-ahaslam@baylibre.com>
Now that all ohci users are are using a regulator, we can
remove the platform callbacks and data.
potpgt is no longer necessary as a power on delay time can
be specified for the regulator itself.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 84 ++-----------------------------
include/linux/platform_data/usb-davinci.h | 20 --------
2 files changed, 5 insertions(+), 99 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 0a4b885..3dcbf1f 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -89,12 +89,8 @@ static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int ret;
- if (hub && hub->set_power)
- return hub->set_power(1, on);
-
if (!da8xx_ohci->vbus_reg)
return 0;
@@ -121,11 +117,6 @@ static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
static int ohci_da8xx_get_power(struct usb_hcd *hcd)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
- struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
-
- if (hub && hub->get_power)
- return hub->get_power(1);
if (da8xx_ohci->vbus_reg)
return regulator_is_enabled(da8xx_ohci->vbus_reg);
@@ -137,13 +128,9 @@ static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
unsigned int flags;
int ret;
- if (hub && hub->get_oci)
- return hub->get_oci(1);
-
if (!da8xx_ohci->vbus_reg)
return 0;
@@ -159,29 +146,9 @@ static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
return 0;
}
-static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
+static int ohci_da8xx_has_regulator(struct usb_hcd *hcd)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
- struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
-
- if (hub && hub->set_power)
- return 1;
-
- if (da8xx_ohci->vbus_reg)
- return 1;
-
- return 0;
-}
-
-static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
-{
- struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
- struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
-
- if (hub && hub->get_oci)
- return 1;
if (da8xx_ohci->vbus_reg)
return 1;
@@ -189,30 +156,9 @@ static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
return 0;
}
-static int ohci_da8xx_has_potpgt(struct usb_hcd *hcd)
-{
- struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
-
- if (hub && hub->potpgt)
- return 1;
-
- return 0;
-}
-
/*
* Handle the port over-current indicator change.
*/
-static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
- unsigned port)
-{
- ocic_mask |= 1 << port;
-
- /* Once over-current is detected, the port needs to be powered down */
- if (hub->get_oci(port) > 0)
- hub->set_power(port, 0);
-}
-
static int ohci_da8xx_regulator_event(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -233,12 +179,9 @@ static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
{
struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int ret = 0;
- if (hub && hub->ocic_notify)
- ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
- else if (da8xx_ohci->vbus_reg) {
+ if (da8xx_ohci->vbus_reg) {
da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
ret = devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
&da8xx_ohci->nb);
@@ -250,19 +193,9 @@ static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
return ret;
}
-static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
-{
- struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
-
- if (hub && hub->ocic_notify)
- hub->ocic_notify(NULL);
-}
-
static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int result;
u32 rh_a;
@@ -291,20 +224,14 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
* register's default value, so that ohci_hub_control() could return
* the correct hub descriptor...
*/
- rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (ohci_da8xx_has_set_power(hcd)) {
+ if (ohci_da8xx_has_regulator(hcd)) {
+ rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
- }
- if (ohci_da8xx_has_oci(hcd)) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
+ ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
}
- if (ohci_da8xx_has_potpgt(hcd)) {
- rh_a &= ~RH_A_POTPGT;
- rh_a |= hub->potpgt << 24;
- }
- ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
}
@@ -479,7 +406,6 @@ static int ohci_da8xx_remove(struct platform_device *pdev)
{
struct usb_hcd *hcd = platform_get_drvdata(pdev);
- ohci_da8xx_unregister_notify(hcd);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index 0926e99..58f4be0 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -11,26 +11,6 @@
#ifndef __ASM_ARCH_USB_H
#define __ASM_ARCH_USB_H
-struct da8xx_ohci_root_hub;
-
-typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub,
- unsigned port);
-
-/* Passed as the platform data to the OHCI driver */
-struct da8xx_ohci_root_hub {
- /* Switch the port power on/off */
- int (*set_power)(unsigned port, int on);
- /* Read the port power status */
- int (*get_power)(unsigned port);
- /* Read the port over-current indicator */
- int (*get_oci)(unsigned port);
- /* Over-current indicator change notification (pass NULL to disable) */
- int (*ocic_notify)(da8xx_ocic_handler_t handler);
-
- /* Time from power on to power good (in 2 ms units) */
- u8 potpgt;
-};
-
void davinci_setup_usb(unsigned mA, unsigned potpgt_ms);
#endif /* ifndef __ASM_ARCH_USB_H */
--
2.10.1
^ permalink raw reply related
* [PATCH 0/2] [PART 3/4] USB: ohci-da8xx: Remove platform callbacks
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
There are no more users of the platform callbacks as
all of them have been converted to use a regulator.
We can now remove the plafrom callbacks from the driver and the platform
data structure.
DEPENDENCIES:
1. [PATCH 0/3] fix ohci phy name
https://lkml.org/lkml/2016/11/2/208
2. [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers
https://lkml.org/lkml/2016/11/3/188
3. [PATCH v4 0/3] [PART 1/4] USB: ohci-da8xx: Allow a regulator for VBUS and over current
4. [PATCH 0/3] [PART 2/4] ARM: davinci: OHCI: Use a regulator instead of callbacks
A branch with all the dependencies can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt-v4
Axel Haslam (2):
USB: ohci: da8xx: Remove ohci platform callbacks
USB: ohci: da8xx: use a flag instead of mask for ocic
drivers/usb/host/ohci-da8xx.c | 101 ++++--------------------------
include/linux/platform_data/usb-davinci.h | 20 ------
2 files changed, 12 insertions(+), 109 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCH 3/3] ARM: davinci: remove ohci platform usage
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185738.17571-1-ahaslam@baylibre.com>
As all users of ohci platform data have been converted
to use a regulator, we dont need to pass platform
data to register the ohci device anymore.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 2 +-
arch/arm/mach-davinci/board-omapl138-hawk.c | 2 +-
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 3 +--
4 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 16a401a..cb67885 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -129,7 +129,7 @@ static __init void da830_evm_usb_init(void)
if (ret)
pr_warn("fail to add ohci regulator\n");
- ret = da8xx_register_usb11(NULL);
+ ret = da8xx_register_usb11();
if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
}
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index a252404..cbe7324 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -197,7 +197,7 @@ static __init void omapl138_hawk_usb_init(void)
pr_warn("%s: USB PHY registration failed: %d\n",
__func__, ret);
- ret = da8xx_register_usb11(NULL);
+ ret = da8xx_register_usb11();
if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n",
__func__, ret);
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 43322be..6096402 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -91,7 +91,7 @@ int da8xx_register_spi_bus(int instance, unsigned num_chipselect);
int da8xx_register_watchdog(void);
int da8xx_register_usb_phy(void);
int da8xx_register_usb20(unsigned mA, unsigned potpgt);
-int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
+int da8xx_register_usb11(void);
int da8xx_register_usb_refclkin(int rate);
int da8xx_register_usb20_phy_clk(bool use_usb_refclkin);
int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index c6feecf..4ea91bb 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -119,9 +119,8 @@ static struct platform_device da8xx_usb11_device = {
.resource = da8xx_usb11_resources,
};
-int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata)
+int __init da8xx_register_usb11(void)
{
- da8xx_usb11_device.dev.platform_data = pdata;
return platform_device_register(&da8xx_usb11_device);
}
--
2.10.1
^ permalink raw reply related
* [PATCH 2/3] ARM: davinci: hawk: Remove vbus and over current gpios
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185738.17571-1-ahaslam@baylibre.com>
The hawk board VBUS is fixed to a 5v source, and the over
current pin is actually not connected to the SoC.
Do not reseve these gpios for OHCI as they are not related
to usb.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-omapl138-hawk.c | 99 ++---------------------------
1 file changed, 4 insertions(+), 95 deletions(-)
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index a4e8726..a252404 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -28,9 +28,6 @@
#define DA850_HAWK_MMCSD_CD_PIN GPIO_TO_PIN(3, 12)
#define DA850_HAWK_MMCSD_WP_PIN GPIO_TO_PIN(3, 13)
-#define DA850_USB1_VBUS_PIN GPIO_TO_PIN(2, 4)
-#define DA850_USB1_OC_PIN GPIO_TO_PIN(6, 13)
-
static short omapl138_hawk_mii_pins[] __initdata = {
DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
DA850_MII_TXD_2, DA850_MII_TXD_1, DA850_MII_TXD_0, DA850_MII_RXER,
@@ -181,76 +178,10 @@ static __init void omapl138_hawk_mmc_init(void)
gpio_free(DA850_HAWK_MMCSD_CD_PIN);
}
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
-static da8xx_ocic_handler_t hawk_usb_ocic_handler;
-
-static const short da850_hawk_usb11_pins[] = {
- DA850_GPIO2_4, DA850_GPIO6_13,
- -1
-};
-
-static int hawk_usb_set_power(unsigned port, int on)
-{
- gpio_set_value(DA850_USB1_VBUS_PIN, on);
- return 0;
-}
-
-static int hawk_usb_get_power(unsigned port)
-{
- return gpio_get_value(DA850_USB1_VBUS_PIN);
-}
-
-static int hawk_usb_get_oci(unsigned port)
-{
- return !gpio_get_value(DA850_USB1_OC_PIN);
-}
-
-static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(DA850_USB1_OC_PIN);
- int error = 0;
-
- if (handler != NULL) {
- hawk_usb_ocic_handler = handler;
-
- error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
- IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch "
- "over-current indicator changes\n", __func__);
- } else {
- free_irq(irq, NULL);
- }
- return error;
-}
-
-static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
- .set_power = hawk_usb_set_power,
- .get_power = hawk_usb_get_power,
- .get_oci = hawk_usb_get_oci,
- .ocic_notify = hawk_usb_ocic_notify,
- /* TPS2087 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
-};
-
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
-{
- hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata, 1);
- return IRQ_HANDLED;
-}
-
static __init void omapl138_hawk_usb_init(void)
{
int ret;
- ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
- if (ret) {
- pr_warn("%s: USB 1.1 PinMux setup failed: %d\n", __func__, ret);
- return;
- }
-
ret = da8xx_register_usb20_phy_clk(false);
if (ret)
pr_warn("%s: USB 2.0 PHY CLK registration failed: %d\n",
@@ -266,34 +197,12 @@ static __init void omapl138_hawk_usb_init(void)
pr_warn("%s: USB PHY registration failed: %d\n",
__func__, ret);
- ret = gpio_request_one(DA850_USB1_VBUS_PIN,
- GPIOF_DIR_OUT, "USB1 VBUS");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "power control: %d\n", __func__, ret);
- return;
- }
-
- ret = gpio_request_one(DA850_USB1_OC_PIN,
- GPIOF_DIR_IN, "USB1 OC");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "over-current indicator: %d\n", __func__, ret);
- goto usb11_setup_oc_fail;
- }
-
- ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
- if (ret) {
- pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
- goto usb11_setup_fail;
- }
+ ret = da8xx_register_usb11(NULL);
+ if (ret)
+ pr_warn("%s: USB 1.1 registration failed: %d\n",
+ __func__, ret);
return;
-
-usb11_setup_fail:
- gpio_free(DA850_USB1_OC_PIN);
-usb11_setup_oc_fail:
- gpio_free(DA850_USB1_VBUS_PIN);
}
static __init void omapl138_hawk_init(void)
--
2.10.1
^ permalink raw reply related
* [PATCH 1/3] ARM: davinci: da830: Handle vbus with a regulator
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185738.17571-1-ahaslam@baylibre.com>
The usb driver can now take a regulator instead of the platform
callbacks for vbus handling. Lets use a regulator so we can remove
the callbacks in a later patch.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 108 +++++++++++---------------------
1 file changed, 38 insertions(+), 70 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index 5db0901..16a401a 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -14,6 +14,7 @@
#include <linux/console.h>
#include <linux/interrupt.h>
#include <linux/gpio.h>
+#include <linux/gpio/machine.h>
#include <linux/platform_device.h>
#include <linux/i2c.h>
#include <linux/i2c/pcf857x.h>
@@ -28,6 +29,7 @@
#include <linux/platform_data/spi-davinci.h>
#include <linux/platform_data/usb-davinci.h>
#include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -38,72 +40,48 @@
#include <mach/da8xx.h>
#define DA830_EVM_PHY_ID ""
-/*
- * USB1 VBUS is controlled by GPIO1[15], over-current is reported on GPIO2[4].
- */
-#define ON_BD_USB_DRV GPIO_TO_PIN(1, 15)
-#define ON_BD_USB_OVC GPIO_TO_PIN(2, 4)
static const short da830_evm_usb11_pins[] = {
DA830_GPIO1_15, DA830_GPIO2_4,
-1
};
-static da8xx_ocic_handler_t da830_evm_usb_ocic_handler;
-
-static int da830_evm_usb_set_power(unsigned port, int on)
-{
- gpio_set_value(ON_BD_USB_DRV, on);
- return 0;
-}
+static struct regulator_consumer_supply usb_ohci_consumer_supply =
+ REGULATOR_SUPPLY("vbus", "ohci-da8xx");
-static int da830_evm_usb_get_power(unsigned port)
-{
- return gpio_get_value(ON_BD_USB_DRV);
-}
-
-static int da830_evm_usb_get_oci(unsigned port)
-{
- return !gpio_get_value(ON_BD_USB_OVC);
-}
-
-static irqreturn_t da830_evm_usb_ocic_irq(int, void *);
-
-static int da830_evm_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(ON_BD_USB_OVC);
- int error = 0;
-
- if (handler != NULL) {
- da830_evm_usb_ocic_handler = handler;
-
- error = request_irq(irq, da830_evm_usb_ocic_irq,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch over-current indicator changes\n",
- __func__);
- } else
- free_irq(irq, NULL);
-
- return error;
-}
+static struct regulator_init_data usb_ohci_initdata = {
+ .consumer_supplies = &usb_ohci_consumer_supply,
+ .num_consumer_supplies = 1,
+ .constraints = {
+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ },
+};
-static struct da8xx_ohci_root_hub da830_evm_usb11_pdata = {
- .set_power = da830_evm_usb_set_power,
- .get_power = da830_evm_usb_get_power,
- .get_oci = da830_evm_usb_get_oci,
- .ocic_notify = da830_evm_usb_ocic_notify,
+static struct fixed_voltage_config usb_ohci_config = {
+ .supply_name = "vbus",
+ .microvolts = 5000000,
+ .gpio = GPIO_TO_PIN(1, 15),
+ .enable_high = 1,
+ .enabled_at_boot = 0,
+ .init_data = &usb_ohci_initdata,
+};
- /* TPS2065 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
+static struct platform_device da8xx_usb11_regulator = {
+ .name = "reg-fixed-voltage",
+ .id = 0,
+ .dev = {
+ .platform_data = &usb_ohci_config,
+ },
};
-static irqreturn_t da830_evm_usb_ocic_irq(int irq, void *dev_id)
-{
- da830_evm_usb_ocic_handler(&da830_evm_usb11_pdata, 1);
- return IRQ_HANDLED;
-}
+static struct gpiod_lookup_table usb11_gpios_table = {
+ .dev_id = "reg-fixed-voltage.0",
+ .table = {
+ /* gpio chip 1 contains gpio range 32-63 */
+ GPIO_LOOKUP("davinci_gpio.1", 4, "over-current",
+ GPIO_ACTIVE_LOW),
+ },
+};
static __init void da830_evm_usb_init(void)
{
@@ -145,23 +123,13 @@ static __init void da830_evm_usb_init(void)
return;
}
- ret = gpio_request(ON_BD_USB_DRV, "ON_BD_USB_DRV");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port power control: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_output(ON_BD_USB_DRV, 0);
+ gpiod_add_lookup_table(&usb11_gpios_table);
- ret = gpio_request(ON_BD_USB_OVC, "ON_BD_USB_OVC");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port over-current indicator: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_input(ON_BD_USB_OVC);
+ ret = platform_device_register(&da8xx_usb11_regulator);
+ if (ret)
+ pr_warn("fail to add ohci regulator\n");
- ret = da8xx_register_usb11(&da830_evm_usb11_pdata);
+ ret = da8xx_register_usb11(NULL);
if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
}
--
2.10.1
^ permalink raw reply related
* [PATCH 0/3] [PART 2/4] ARM: davinci: OHCI: Use a regulator instead of callbacks
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
With the ultimate goal to able to probe the ohci driver form DT,
convert users of OHCI pdata to use a regulator instead of
passing platform function pointers. This will help to remove the
platform callbacks in a future series.
DEPENDENCIES:
1. [PATCH 0/3] fix ohci phy name
https://lkml.org/lkml/2016/11/2/208
2. [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers
https://lkml.org/lkml/2016/11/3/188
3. [PATCH v4 0/3] [PART 1/4] USB: ohci-da8xx: Allow a regulator for VBUS and over current
A branch with all the dependencies can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt-v4
Axel Haslam (3):
ARM: davinci: da830: Handle vbus with a regulator
ARM: davinci: hawk: Remove vbus and over current gpios
ARM: davinci: remove ohci platform usage
arch/arm/mach-davinci/board-da830-evm.c | 108 ++++++++++------------------
arch/arm/mach-davinci/board-omapl138-hawk.c | 99 ++-----------------------
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 3 +-
4 files changed, 44 insertions(+), 168 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCH v4 3/3] USB: ohci: da8xx: Allow a regulator to handle VBUS
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185723.17518-1-ahaslam@baylibre.com>
We need to remove the platform callbacks to be able to probe
the driver using device tree. Using a regulator to handle VBUS will
eliminate the need for these callbacks once all users are converted
to use a regulator.
The regulator equivalents to the platform callbacks are:
set_power -> regulator_enable/regulator_disable
get_power -> regulator_is_enabled
get_oci -> regulator_get_error_flags
ocic_notify -> regulator event notification
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 97 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 95 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 9ed43c7..0a4b885 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/regulator/consumer.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
#include <asm/unaligned.h>
@@ -36,8 +37,12 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
struct da8xx_ohci_hcd {
+ struct usb_hcd *hcd;
struct clk *usb11_clk;
struct phy *usb11_phy;
+ struct regulator *vbus_reg;
+ struct notifier_block nb;
+ unsigned int is_powered;
};
#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
@@ -82,56 +87,105 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ int ret;
if (hub && hub->set_power)
return hub->set_power(1, on);
+ if (!da8xx_ohci->vbus_reg)
+ return 0;
+
+ if (on && !da8xx_ohci->is_powered) {
+ ret = regulator_enable(da8xx_ohci->vbus_reg);
+ if (ret) {
+ dev_err(dev, "Fail to enable regulator: %d\n", ret);
+ return ret;
+ }
+ da8xx_ohci->is_powered = 1;
+
+ } else if (!on && da8xx_ohci->is_powered) {
+ ret = regulator_disable(da8xx_ohci->vbus_reg);
+ if (ret) {
+ dev_err(dev, "Fail to disable regulator: %d\n", ret);
+ return ret;
+ }
+ da8xx_ohci->is_powered = 0;
+ }
+
return 0;
}
static int ohci_da8xx_get_power(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->get_power)
return hub->get_power(1);
+ if (da8xx_ohci->vbus_reg)
+ return regulator_is_enabled(da8xx_ohci->vbus_reg);
+
return 1;
}
static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ unsigned int flags;
+ int ret;
if (hub && hub->get_oci)
return hub->get_oci(1);
+ if (!da8xx_ohci->vbus_reg)
+ return 0;
+
+ ret = regulator_get_error_flags(da8xx_ohci->vbus_reg, &flags);
+ if (ret) {
+ dev_err(dev, "could not get regulator error flags: %d\n", ret);
+ return ret;
+ }
+
+ if (flags && REGULATOR_ERROR_OVER_CURRENT)
+ return 1;
+
return 0;
}
static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->set_power)
return 1;
+ if (da8xx_ohci->vbus_reg)
+ return 1;
+
return 0;
}
static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
if (hub && hub->get_oci)
return 1;
+ if (da8xx_ohci->vbus_reg)
+ return 1;
+
return 0;
}
@@ -159,15 +213,41 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
}
+static int ohci_da8xx_regulator_event(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct da8xx_ohci_hcd *da8xx_ohci =
+ container_of(nb, struct da8xx_ohci_hcd, nb);
+ struct device *dev = da8xx_ohci->hcd->self.controller;
+
+ if (event & REGULATOR_EVENT_OVER_CURRENT) {
+ dev_warn(dev, "over current event\n");
+ ocic_mask |= 1;
+ ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
+ }
+
+ return 0;
+}
+
static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+ int ret = 0;
if (hub && hub->ocic_notify)
- return hub->ocic_notify(ohci_da8xx_ocic_handler);
+ ret = hub->ocic_notify(ohci_da8xx_ocic_handler);
+ else if (da8xx_ohci->vbus_reg) {
+ da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
+ ret = devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
+ &da8xx_ohci->nb);
+ }
- return 0;
+ if (ret)
+ dev_err(dev, "Failed to register notifier: %d\n", ret);
+
+ return ret;
}
static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
@@ -330,6 +410,7 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
return -ENOMEM;
da8xx_ohci = to_da8xx_ohci(hcd);
+ da8xx_ohci->hcd = hcd;
da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
if (IS_ERR(da8xx_ohci->usb11_clk)) {
@@ -347,6 +428,18 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
goto err;
}
+ da8xx_ohci->vbus_reg = devm_regulator_get_optional(&pdev->dev, "vbus");
+ if (IS_ERR(da8xx_ohci->vbus_reg)) {
+ error = PTR_ERR(da8xx_ohci->vbus_reg);
+ if (error == -ENODEV)
+ da8xx_ohci->vbus_reg = NULL;
+ else {
+ dev_err(&pdev->dev,
+ "Failed to get regulator: %d\n", error);
+ goto err;
+ }
+ }
+
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(hcd->regs)) {
--
2.10.1
^ permalink raw reply related
* [PATCH v4 2/3] USB: ohci: da8xx: Prepare to remove platform callbacks
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185723.17518-1-ahaslam@baylibre.com>
Wrap the platform data callbacks into separate functions.
This will help migrate to using a regulator by providing a well defined
place to implement the regulator functions while the platform calls
are still in place and users have not been converted.
The platform callbacks will be removed on a following patch.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 125 ++++++++++++++++++++++++++++++++++--------
1 file changed, 102 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 0442c64..9ed43c7 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -80,6 +80,72 @@ static void ohci_da8xx_disable(struct usb_hcd *hcd)
clk_disable_unprepare(da8xx_ohci->usb11_clk);
}
+static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->set_power)
+ return hub->set_power(1, on);
+
+ return 0;
+}
+
+static int ohci_da8xx_get_power(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_power)
+ return hub->get_power(1);
+
+ return 1;
+}
+
+static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_oci)
+ return hub->get_oci(1);
+
+ return 0;
+}
+
+static int ohci_da8xx_has_set_power(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->set_power)
+ return 1;
+
+ return 0;
+}
+
+static int ohci_da8xx_has_oci(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->get_oci)
+ return 1;
+
+ return 0;
+}
+
+static int ohci_da8xx_has_potpgt(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->potpgt)
+ return 1;
+
+ return 0;
+}
+
/*
* Handle the port over-current indicator change.
*/
@@ -93,6 +159,26 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
hub->set_power(port, 0);
}
+static int ohci_da8xx_register_notify(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->ocic_notify)
+ return hub->ocic_notify(ohci_da8xx_ocic_handler);
+
+ return 0;
+}
+
+static void ohci_da8xx_unregister_notify(struct usb_hcd *hcd)
+{
+ struct device *dev = hcd->self.controller;
+ struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
+
+ if (hub && hub->ocic_notify)
+ hub->ocic_notify(NULL);
+}
+
static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
@@ -126,16 +212,18 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
* the correct hub descriptor...
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (hub->set_power) {
+ if (ohci_da8xx_has_set_power(hcd)) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
}
- if (hub->get_oci) {
+ if (ohci_da8xx_has_oci(hcd)) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
- rh_a &= ~RH_A_POTPGT;
- rh_a |= hub->potpgt << 24;
+ if (ohci_da8xx_has_potpgt(hcd)) {
+ rh_a &= ~RH_A_POTPGT;
+ rh_a |= hub->potpgt << 24;
+ }
ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
@@ -168,7 +256,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int temp;
switch (typeReq) {
@@ -182,11 +269,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (hub->get_power && hub->get_power(wIndex) == 0)
+ if (!ohci_da8xx_get_power(hcd))
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (hub->get_oci && hub->get_oci(wIndex) > 0)
+ if (ohci_da8xx_get_oci(hcd))
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
@@ -211,10 +298,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- if (!hub->set_power)
- return -EPIPE;
-
- return hub->set_power(wIndex, temp) ? -EPIPE : 0;
+ return ohci_da8xx_set_power(hcd, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
@@ -236,15 +320,10 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
-
- if (hub == NULL)
- return -ENODEV;
-
hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
dev_name(&pdev->dev));
if (!hcd)
@@ -290,12 +369,13 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
device_wakeup_enable(hcd->self.controller);
- if (hub->ocic_notify) {
- error = hub->ocic_notify(ohci_da8xx_ocic_handler);
- if (!error)
- return 0;
- }
+ error = ohci_da8xx_register_notify(hcd);
+ if (error)
+ goto err_remove_hcd;
+
+ return 0;
+err_remove_hcd:
usb_remove_hcd(hcd);
err:
usb_put_hcd(hcd);
@@ -305,9 +385,8 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
static int ohci_da8xx_remove(struct platform_device *pdev)
{
struct usb_hcd *hcd = platform_get_drvdata(pdev);
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
- hub->ocic_notify(NULL);
+ ohci_da8xx_unregister_notify(hcd);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
--
2.10.1
^ permalink raw reply related
* [PATCH v4 1/3] USB: ohci: da8xx: use ohci priv data instead of globals
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108185723.17518-1-ahaslam@baylibre.com>
Instead of global variables, use the extra_priv_size of
the ohci driver.
We cannot yet move the ocic mask because this is used on
the interrupt handler which is registerded through platform
data and does not have an hcd pointer. This will be moved
on a later patch.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 72 +++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 429d58b..0442c64 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -35,43 +35,49 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
u16 wValue, u16 wIndex, char *buf, u16 wLength);
static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
-static struct clk *usb11_clk;
-static struct phy *usb11_phy;
+struct da8xx_ohci_hcd {
+ struct clk *usb11_clk;
+ struct phy *usb11_phy;
+};
+#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
/* Over-current indicator change bitmask */
static volatile u16 ocic_mask;
-static int ohci_da8xx_enable(void)
+static int ohci_da8xx_enable(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret;
- ret = clk_prepare_enable(usb11_clk);
+ ret = clk_prepare_enable(da8xx_ohci->usb11_clk);
if (ret)
return ret;
- ret = phy_init(usb11_phy);
+ ret = phy_init(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_init;
- ret = phy_power_on(usb11_phy);
+ ret = phy_power_on(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_power_on;
return 0;
err_phy_power_on:
- phy_exit(usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
err_phy_init:
- clk_disable_unprepare(usb11_clk);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
return ret;
}
-static void ohci_da8xx_disable(void)
+static void ohci_da8xx_disable(struct usb_hcd *hcd)
{
- phy_power_off(usb11_phy);
- phy_exit(usb11_phy);
- clk_disable_unprepare(usb11_clk);
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+ phy_power_off(da8xx_ohci->usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
}
/*
@@ -97,7 +103,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
dev_dbg(dev, "starting USB controller\n");
- result = ohci_da8xx_enable();
+ result = ohci_da8xx_enable(hcd);
if (result < 0)
return result;
@@ -109,7 +115,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
result = ohci_setup(hcd);
if (result < 0) {
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
return result;
}
@@ -231,6 +237,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
+ struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
@@ -238,25 +245,29 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
if (hub == NULL)
return -ENODEV;
- usb11_clk = devm_clk_get(&pdev->dev, "usb11");
- if (IS_ERR(usb11_clk)) {
- if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
+ hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
+ dev_name(&pdev->dev));
+ if (!hcd)
+ return -ENOMEM;
+
+ da8xx_ohci = to_da8xx_ohci(hcd);
+
+ da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
+ if (IS_ERR(da8xx_ohci->usb11_clk)) {
+ if (PTR_ERR(da8xx_ohci->usb11_clk) != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get clock.\n");
- return PTR_ERR(usb11_clk);
+ error = PTR_ERR(da8xx_ohci->usb11_clk);
+ goto err;
}
- usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
- if (IS_ERR(usb11_phy)) {
- if (PTR_ERR(usb11_phy) != -EPROBE_DEFER)
+ da8xx_ohci->usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
+ if (IS_ERR(da8xx_ohci->usb11_phy)) {
+ if (PTR_ERR(da8xx_ohci->usb11_phy) != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get phy.\n");
- return PTR_ERR(usb11_phy);
+ error = PTR_ERR(da8xx_ohci->usb11_phy);
+ goto err;
}
- hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
- dev_name(&pdev->dev));
- if (!hcd)
- return -ENOMEM;
-
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(hcd->regs)) {
@@ -321,7 +332,7 @@ static int ohci_da8xx_suspend(struct platform_device *pdev,
if (ret)
return ret;
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
hcd->state = HC_STATE_SUSPENDED;
return ret;
@@ -337,7 +348,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
msleep(5);
ohci->next_statechange = jiffies;
- ret = ohci_da8xx_enable();
+ ret = ohci_da8xx_enable(hcd);
if (ret)
return ret;
@@ -349,7 +360,8 @@ static int ohci_da8xx_resume(struct platform_device *dev)
#endif
static const struct ohci_driver_overrides da8xx_overrides __initconst = {
- .reset = ohci_da8xx_reset,
+ .reset = ohci_da8xx_reset,
+ .extra_priv_size = sizeof(struct da8xx_ohci_hcd),
};
/*
--
2.10.1
^ permalink raw reply related
* [PATCH v4 0/3] [PART 1/4] USB: ohci-da8xx: Allow a regulator for VBUS and over current
From: Axel Haslam @ 2016-11-08 18:57 UTC (permalink / raw)
To: linux-arm-kernel
This is the first of 4 series of patches to convert the
da8xx-ohci driver to use a regulator and probe form device tree
To be able to use device tree to probe the driver, we need to remove
the platform callbacks that are handling vbus and over current.
These patches prepare the stage by allowing to use a regulator
instead of the platform callbacks.
DEPENDENCIES:
1. [PATCH 0/3] fix ohci phy name
https://lkml.org/lkml/2016/11/2/208
2. [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers
https://lkml.org/lkml/2016/11/3/188
A branch with all the dependencies can be found here:
https://github.com/axelhaslamx/linux-axel/commits/ohci-da8xx-dt-v4
Changes from v3->v4
* separate the series into smaller series for driver and arch/arm code,
to ease review and merging to different trees.
Changes form v2->v3
* drop patches that have been integrated to arch/arm
* drop regulator patches which will be integrated through regulator tree
* use of the accepted regulator API to get over current status
* better patch separation with the use of wrappers
Changes from v1->v2
* Rebased and added patch to make ohci a separate driver
* Use a regulator instead of handling Gpios (David Lechner)
* Add an over current mode to regulator framework
* Fixed regulator is able to register for and over current irq
* Added patch by Alexandre to remove build warnings
* Moved global variables into private hcd structure.
Axel Haslam (3):
USB: ohci: da8xx: use ohci priv data instead of globals
USB: ohci: da8xx: Prepare to remove platform callbacks
USB: ohci: da8xx: Allow a regulator to handle VBUS
drivers/usb/host/ohci-da8xx.c | 284 ++++++++++++++++++++++++++++++++++--------
1 file changed, 234 insertions(+), 50 deletions(-)
--
2.10.1
^ permalink raw reply
* [PATCH v3 3/3] ARM: dmaengine: sun6i: share the dma driver with sun50i
From: Maxime Ripard @ 2016-11-08 18:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161107182840.GA3711@arx12>
On Tue, Nov 08, 2016 at 02:28:40AM +0800, Hao Zhang wrote:
> According to the datasheet, the dma of A64 support 8/16/32/64 bits
> so, we can add the condition of device compatible in convert_buswidth
> function and other place to determine the device whether is for A64,
> and then accept the 8 bytes bus width to it.
>
> Signed-off-by: Hao Zhang <hao5781286@gmail.com>
> ---
> drivers/dma/sun6i-dma.c | 43 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 8346199..8a95a1a 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -247,13 +247,17 @@ static inline s8 convert_burst(u32 maxburst)
> }
> }
>
> -static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
> +static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width,
> + struct sun6i_dma_dev *sdev)
> {
> - if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
> - (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
> + if (((addr_width >= DMA_SLAVE_BUSWIDTH_1_BYTE) &&
> + (addr_width <= DMA_SLAVE_BUSWIDTH_4_BYTES)) ||
> + ((addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES) &&
> + (of_device_is_compatible(sdev->slave.dev->of_node,
> + "allwinner,sun50i-a64-dma"))))
> + return addr_width >> 1;
> + else
Just like for the burst (https://lkml.org/lkml/2016/10/4/367) I think
this should be taken care of in the the framework's
dmaengine_slave_config function.
This is quite easy to do in the width case, since you just have to
test whether what has been set in the dma_device has support for the
burst give in the configuration.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/df9a830d/attachment.sig>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox