* [PATCH] regulator: axp20x: Fix axp809 ldo_io registration error on cold boot
From: Chen-Yu Tsai @ 2016-11-11 3:12 UTC (permalink / raw)
To: linux-arm-kernel
The maximum supported voltage for ldo_io# is 3.3V, but on cold boot
the selector comes up at 0x1f, which maps to 3.8V. This was previously
corrected by Allwinner's U-boot, which set all regulators on the PMICs
to some pre-configured voltage. With recent progress in U-boot SPL
support, this is no longer the case. In any case we should handle
this quirk in the kernel driver as well.
This invalid setting causes _regulator_get_voltage() to fail with -EINVAL
which causes regulator registration to fail when constrains are used:
[ 1.054181] vcc-pg: failed to get the current voltage(-22)
[ 1.059670] axp20x-regulator axp20x-regulator.0: Failed to register ldo_io0
[ 1.069749] axp20x-regulator: probe of axp20x-regulator.0 failed with error -22
This commits makes the axp20x regulator driver accept the 0x1f register
value, fixing this.
The datasheet does not guarantee reliable operation above 3.3V, so on
boards where this regulator is used the regulator-max-microvolt setting
must be 3.3V or less.
This is essentially the same as the commit f40d4896bf32 ("regulator:
axp20x: Fix axp22x ldo_io registration error on cold boot") for AXP22x
PMICs.
Fixes: a51f9f4622a3 ("regulator: axp20x: support AXP809 variant")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/regulator/axp20x-regulator.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 54382ef902c6..e6a512ebeae2 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -337,10 +337,18 @@ static const struct regulator_desc axp809_regulators[] = {
AXP22X_ELDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(1)),
AXP_DESC(AXP809, ELDO3, "eldo3", "eldoin", 700, 3300, 100,
AXP22X_ELDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(2)),
- AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+ /*
+ * Note the datasheet only guarantees reliable operation up to
+ * 3.3V, this needs to be enforced via dts provided constraints
+ */
+ AXP_DESC_IO(AXP809, LDO_IO0, "ldo_io0", "ips", 700, 3800, 100,
AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
- AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
+ /*
+ * Note the datasheet only guarantees reliable operation up to
+ * 3.3V, this needs to be enforced via dts provided constraints
+ */
+ AXP_DESC_IO(AXP809, LDO_IO1, "ldo_io1", "ips", 700, 3800, 100,
AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
AXP_DESC_FIXED(AXP809, RTC_LDO, "rtc_ldo", "ips", 1800),
--
2.10.2
^ permalink raw reply related
* [PATCH v6 1/9] drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver
From: Rongrong Zou @ 2016-11-11 3:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOw6vbJTzmMNLXauNjrXE3-fZCus=Q4S8Hm=hyo_GoyC-QOs1A@mail.gmail.com>
Hi Sean,
Thanks for reviewing! All comments is helpful.
? 2016/11/11 1:35, Sean Paul ??:
> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
>> Add DRM master driver for Hisilicon Hibmc SoC which used for
>> Out-of-band management. Blow is the general hardware connection,
>> both the Hibmc and the host CPU are on the same mother board.
>>
>> +----------+ +----------+
>> | | PCIe | Hibmc |
>> |host CPU( |<----->| display |
>> |arm64,x86)| |subsystem |
>> +----------+ +----------+
>>
>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>> ---
>> drivers/gpu/drm/hisilicon/Kconfig | 1 +
>> drivers/gpu/drm/hisilicon/Makefile | 1 +
>> drivers/gpu/drm/hisilicon/hibmc/Kconfig | 7 +
>> drivers/gpu/drm/hisilicon/hibmc/Makefile | 5 +
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 269 ++++++++++++++++++++++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 35 +++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c | 85 +++++++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h | 28 +++
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h | 212 +++++++++++++++++
>> 9 files changed, 643 insertions(+)
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/Kconfig
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/Makefile
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
>> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
>>
>> diff --git a/drivers/gpu/drm/hisilicon/Kconfig b/drivers/gpu/drm/hisilicon/Kconfig
>> index 558c61b..2fd2724 100644
>> --- a/drivers/gpu/drm/hisilicon/Kconfig
>> +++ b/drivers/gpu/drm/hisilicon/Kconfig
>> @@ -2,4 +2,5 @@
>> # hisilicon drm device configuration.
>> # Please keep this list sorted alphabetically
>>
>> +source "drivers/gpu/drm/hisilicon/hibmc/Kconfig"
>> source "drivers/gpu/drm/hisilicon/kirin/Kconfig"
>> diff --git a/drivers/gpu/drm/hisilicon/Makefile b/drivers/gpu/drm/hisilicon/Makefile
>> index e3f6d49..c8155bf 100644
>> --- a/drivers/gpu/drm/hisilicon/Makefile
>> +++ b/drivers/gpu/drm/hisilicon/Makefile
>> @@ -2,4 +2,5 @@
>> # Makefile for hisilicon drm drivers.
>> # Please keep this list sorted alphabetically
>>
>> +obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc/
>> obj-$(CONFIG_DRM_HISI_KIRIN) += kirin/
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>> new file mode 100644
>> index 0000000..a9af90d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>> @@ -0,0 +1,7 @@
>> +config DRM_HISI_HIBMC
>> + tristate "DRM Support for Hisilicon Hibmc"
>> + depends on DRM && PCI
>> +
>> + help
>> + Choose this option if you have a Hisilicon Hibmc soc chipset.
>> + If M is selected the module will be called hibmc-drm.
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> new file mode 100644
>> index 0000000..97cf4a0
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> @@ -0,0 +1,5 @@
>> +ccflags-y := -Iinclude/drm
>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
>> +
>> +obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
>
> nit: Improper spacing here
seems a space was missed, thanks.
>
>> +#obj-y += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> new file mode 100644
>> index 0000000..4669d42
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -0,0 +1,269 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/console.h>
>> +
>> +#include "hibmc_drm_drv.h"
>> +#include "hibmc_drm_regs.h"
>> +#include "hibmc_drm_power.h"
>
> nit: Alphabetize headers
agreed, thanks.
>
>> +
>> +static const struct file_operations hibmc_fops = {
>> + .owner = THIS_MODULE,
>> + .open = drm_open,
>> + .release = drm_release,
>> + .unlocked_ioctl = drm_ioctl,
>> +#ifdef CONFIG_COMPAT
>
> drm_compat_ioctl is now initialized to NULL, so you can remove the #ifdef
>
understood, will remove it next version.
>> + .compat_ioctl = drm_compat_ioctl,
>> +#endif
>> + .poll = drm_poll,
>> + .read = drm_read,
>> + .llseek = no_llseek,
>> +};
>> +
>> +static int hibmc_enable_vblank(struct drm_device *dev, unsigned int pipe)
>> +{
>> + return 0;
>> +}
>> +
>> +static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
>> +{
>> +}
>> +
>> +static struct drm_driver hibmc_driver = {
>> + .fops = &hibmc_fops,
>> + .name = "hibmc",
>> + .date = "20160828",
>> + .desc = "hibmc drm driver",
>> + .major = 1,
>> + .minor = 0,
>> + .get_vblank_counter = drm_vblank_no_hw_counter,
>> + .enable_vblank = hibmc_enable_vblank,
>> + .disable_vblank = hibmc_disable_vblank,
>> +};
>> +
>> +static int hibmc_pm_suspend(struct device *dev)
>> +{
>> + return 0;
>> +}
>> +
>> +static int hibmc_pm_resume(struct device *dev)
>> +{
>> + return 0;
>> +}
>> +
>> +static const struct dev_pm_ops hibmc_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(hibmc_pm_suspend,
>> + hibmc_pm_resume)
>> +};
>> +
>> +static int hibmc_hw_config(struct hibmc_drm_device *hidev)
>> +{
>> + unsigned int reg;
>> +
>> + /* On hardware reset, power mode 0 is default. */
>> + hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_MODE0);
>> +
>> + /* Enable display power gate & LOCALMEM power gate*/
>> + reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
>> + reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
>> + reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
>> + reg |= HIBMC_CURR_GATE_DISPLAY(ON);
>> + reg |= HIBMC_CURR_GATE_LOCALMEM(ON);
>> +
>> + hibmc_set_current_gate(hidev, reg);
>> +
>> + /* Reset the memory controller. If the memory controller
>> + * is not reset in chip,the system might hang when sw accesses
>> + * the memory.The memory should be resetted after
>> + * changing the MXCLK.
>> + */
>> + reg = readl(hidev->mmio + HIBMC_MISC_CTRL);
>> + reg &= ~HIBMC_MSCCTL_LOCALMEM_RESET_MASK;
>> + reg |= HIBMC_MSCCTL_LOCALMEM_RESET(RESET);
>> + writel(reg, hidev->mmio + HIBMC_MISC_CTRL);
>> +
>> + reg &= ~HIBMC_MSCCTL_LOCALMEM_RESET_MASK;
>> + reg |= HIBMC_MSCCTL_LOCALMEM_RESET(NORMAL);
>> +
>> + writel(reg, hidev->mmio + HIBMC_MISC_CTRL);
>> +
>> + /* We can add more initialization as needed. */
>> +
>> + return 0;
>
> Consider using void return type here to simplify error checking in the
> caller, especially since it looks like you aren't checking the return
> code, anyways :)
Yes, you are right. i did not check the return value, so void type
is better, thanks.
>
>> +}
>> +
>> +static int hibmc_hw_map(struct hibmc_drm_device *hidev)
>> +{
>> + struct drm_device *dev = hidev->dev;
>> + struct pci_dev *pdev = dev->pdev;
>> + resource_size_t addr, size, ioaddr, iosize;
>> +
>> + ioaddr = pci_resource_start(pdev, 1);
>> + iosize = MB(2);
>> +
>> + hidev->mmio = ioremap_nocache(ioaddr, iosize);
>
> Use devm_ioremap_nocache to avoid managing the resource directly
agreed, thanks
>
>> +
>
> nit: extra space
>
>> + if (!hidev->mmio) {
>> + DRM_ERROR("Cannot map mmio region\n");
>> + return -ENOMEM;
>> + }
>> +
>> + addr = pci_resource_start(pdev, 0);
>> + size = MB(32);
>
> Pull size and iosize out into #defines with descriptive names
agreed, thanks
>
>> +
>> + hidev->fb_map = ioremap(addr, size);
>
> Use devm_ioremap to avoid managing the resource directly.
agreed, thanks
>
>> + if (!hidev->fb_map) {
>> + DRM_ERROR("Cannot map framebuffer\n");
>> + return -ENOMEM;
>> + }
>> + hidev->fb_base = addr;
>> + hidev->fb_size = size;
>> +
>> + return 0;
>> +}
>> +
>> +static void hibmc_hw_fini(struct hibmc_drm_device *hidev)
>> +{
>> + if (hidev->mmio)
>> + iounmap(hidev->mmio);
>> + if (hidev->fb_map)
>> + iounmap(hidev->fb_map);
>> +}
>
> You don't need this function if you use the devm variants above
yes, it seems more simple :)
>
>> +
>> +static int hibmc_hw_init(struct hibmc_drm_device *hidev)
>> +{
>> + int ret;
>> +
>> + ret = hibmc_hw_map(hidev);
>> + if (ret)
>> + return ret;
>> +
>> + hibmc_hw_config(hidev);
>> +
>> + return 0;
>> +}
>> +
>> +static int hibmc_unload(struct drm_device *dev)
>> +{
>> + struct hibmc_drm_device *hidev = dev->dev_private;
>> +
>> + hibmc_hw_fini(hidev);
>> + dev->dev_private = NULL;
>> + return 0;
>> +}
>> +
>> +static int hibmc_load(struct drm_device *dev, unsigned long flags)
>
> flags isn't used anywhere?
Initially, hibmc_load is assigned to ".load", so second parameter is reserved.
In fact it is not used.
>
>> +{
>> + struct hibmc_drm_device *hidev;
>> + int ret;
>> +
>> + hidev = devm_kzalloc(dev->dev, sizeof(*hidev), GFP_KERNEL);
>> + if (!hidev)
>
> Print error here?
applied, thanks.
>
>> + return -ENOMEM;
>> + dev->dev_private = hidev;
>> + hidev->dev = dev;
>> +
>> + ret = hibmc_hw_init(hidev);
>> + if (ret)
>> + goto err;
>> +
>> + return 0;
>> +
>> +err:
>> + hibmc_unload(dev);
>> + DRM_ERROR("failed to initialize drm driver.\n");
>
> Print the return value
ditto
>
>> + return ret;
>> +}
>> +
>> +static int hibmc_pci_probe(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> +{
>> + struct drm_device *dev;
>> + int ret;
>> +
>> + dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
>> + if (!dev)
>
> Print error here
ditto
>
>> + return -ENOMEM;
>> +
>> + dev->pdev = pdev;
>> + pci_set_drvdata(pdev, dev);
>> +
>> + ret = pci_enable_device(pdev);
>> + if (ret)
>
> and here, and in other failure paths
ditto
>
>> + goto err_free;
>> +
>> + ret = hibmc_load(dev, 0);
>> + if (ret)
>> + goto err_disable;
>> +
>> + ret = drm_dev_register(dev, 0);
>> + if (ret)
>> + goto err_unload;
>> +
>> + return 0;
>> +
>> +err_unload:
>> + hibmc_unload(dev);
>> +err_disable:
>> + pci_disable_device(pdev);
>> +err_free:
>> + drm_dev_unref(dev);
>> +
>> + return ret;
>> +}
>> +
>> +static void hibmc_pci_remove(struct pci_dev *pdev)
>> +{
>> + struct drm_device *dev = pci_get_drvdata(pdev);
>> +
>> + drm_dev_unregister(dev);
>> + hibmc_unload(dev);
>> + drm_dev_unref(dev);
>> +}
>> +
>> +static struct pci_device_id hibmc_pci_table[] = {
>> + {0x19e5, 0x1711, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
>> + {0,}
>> +};
>> +
>> +static struct pci_driver hibmc_pci_driver = {
>> + .name = "hibmc-drm",
>> + .id_table = hibmc_pci_table,
>> + .probe = hibmc_pci_probe,
>> + .remove = hibmc_pci_remove,
>> + .driver.pm = &hibmc_pm_ops,
>> +};
>> +
>> +static int __init hibmc_init(void)
>> +{
>> + return pci_register_driver(&hibmc_pci_driver);
>> +}
>> +
>> +static void __exit hibmc_exit(void)
>> +{
>> + return pci_unregister_driver(&hibmc_pci_driver);
>> +}
>> +
>> +module_init(hibmc_init);
>> +module_exit(hibmc_exit);
>> +
>> +MODULE_DEVICE_TABLE(pci, hibmc_pci_table);
>> +MODULE_AUTHOR("RongrongZou <zourongrong@huawei.com>");
>> +MODULE_DESCRIPTION("DRM Driver for Hisilicon Hibmc");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> new file mode 100644
>> index 0000000..0037341
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -0,0 +1,35 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#ifndef HIBMC_DRM_DRV_H
>> +#define HIBMC_DRM_DRV_H
>> +
>> +#include <drm/drmP.h>
>> +
>> +struct hibmc_drm_device {
>
> nit: Calling this hibmc_drm_priv would probably make things easier to
> read. When I read hibmc_drm_device, it makes me think that it's an
> extension of drm_device, which this isn't.
okay, will replace hibmc_drm_device with hibmc_drm_priv, thanks.
>
>
>> + /* hw */
>> + void __iomem *mmio;
>> + void __iomem *fb_map;
>> + unsigned long fb_base;
>> + unsigned long fb_size;
>> +
>> + /* drm */
>> + struct drm_device *dev;
>> +};
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
>> new file mode 100644
>> index 0000000..1036542
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
>
> I don't think you need a new file for these functions. Just stash them
> in hibmc_drm_drv.c
okay, thanks.
>
>> @@ -0,0 +1,85 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#include "hibmc_drm_drv.h"
>> +#include "hibmc_drm_regs.h"
>> +
>> +/*
>> + * It can operate in one of three modes: 0, 1 or Sleep.
>> + */
>> +void hibmc_set_power_mode(struct hibmc_drm_device *hidev,
>> + unsigned int power_mode)
>> +{
>> + unsigned int control_value = 0;
>> + void __iomem *mmio = hidev->mmio;
>> +
>> + if (power_mode > HIBMC_PW_MODE_CTL_MODE_SLEEP)
>> + return;
>> +
>> + control_value = readl(mmio + HIBMC_POWER_MODE_CTRL);
>> + control_value &= ~HIBMC_PW_MODE_CTL_MODE_MASK;
>> +
>> + control_value |= HIBMC_PW_MODE_CTL_MODE(power_mode) &
>> + HIBMC_PW_MODE_CTL_MODE_MASK;
>> +
>> + /* Set up other fields in Power Control Register */
>> + if (power_mode == HIBMC_PW_MODE_CTL_MODE_SLEEP) {
>> + control_value &= ~HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
>
> You do this in both branches of the conditional
sounds good to me, thanks :)
>
>> + control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(0) &
>> + HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
>> + } else {
>> + control_value &= ~HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
>> + control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(1) &
>> + HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
>> + }
>
> I think you could simplify this by adding a new local.
>
> unsigned int input = 1;
>
> if (power_mode == HIBMC_PW_MODE_CTL_MODE_SLEEP)
> input = 0;
>
> control_value = readl(mmio + HIBMC_POWER_MODE_CTRL);
> control_value &= ~(HIBMC_PW_MODE_CTL_MODE_MASK |
> HIBMC_PW_MODE_CTL_OSC_INPUT_MASK);
> control_value |= HIBMC_PW_MODE_CTL_MODE(power_mode) &
> HIBMC_PW_MODE_CTL_MODE_MASK;
> control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(input) &
> HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
agreed.
>
>
>> + /* Program new power mode. */
>> + writel(control_value, mmio + HIBMC_POWER_MODE_CTRL);
>> +}
>> +
>> +static unsigned int hibmc_get_power_mode(struct hibmc_drm_device *hidev)
>> +{
>> + void __iomem *mmio = hidev->mmio;
>> +
>> + return (readl(mmio + HIBMC_POWER_MODE_CTRL) &
>> + HIBMC_PW_MODE_CTL_MODE_MASK) >> HIBMC_PW_MODE_CTL_MODE_SHIFT;
>
> nit: You're doing a lot of work in the return statement here.
so i need to define an extra variable.
>
>> +}
>> +
>> +void hibmc_set_current_gate(struct hibmc_drm_device *hidev, unsigned int gate)
>> +{
>> + unsigned int gate_reg;
>> + unsigned int mode;
>> + void __iomem *mmio = hidev->mmio;
>> +
>> + /* Get current power mode. */
>
> nit: try to avoid comments that don't add value
okay, thanks.
>
>> + mode = hibmc_get_power_mode(hidev);
>> +
>> + switch (mode) {
>> + case HIBMC_PW_MODE_CTL_MODE_MODE0:
>> + gate_reg = HIBMC_MODE0_GATE;
>> + break;
>> +
>> + case HIBMC_PW_MODE_CTL_MODE_MODE1:
>> + gate_reg = HIBMC_MODE1_GATE;
>> + break;
>> +
>> + default:
>> + gate_reg = HIBMC_MODE0_GATE;
>> + break;
>> + }
>> + writel(gate, mmio + gate_reg);
>> +}
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
>> new file mode 100644
>> index 0000000..e20e1aa
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
>> @@ -0,0 +1,28 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#ifndef HIBMC_DRM_POWER_H
>> +#define HIBMC_DRM_POWER_H
>> +
>> +#include "hibmc_drm_drv.h"
>> +
>> +void hibmc_set_power_mode(struct hibmc_drm_device *hidev,
>> + unsigned int power_mode);
>> +void hibmc_set_current_gate(struct hibmc_drm_device *hidev,
>> + unsigned int gate);
>> +#endif
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
>> new file mode 100644
>> index 0000000..9c804ca
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
>> @@ -0,0 +1,212 @@
>> +/* Hisilicon Hibmc SoC drm driver
>> + *
>> + * Based on the bochs drm driver.
>> + *
>> + * Copyright (c) 2016 Huawei Limited.
>> + *
>> + * Author:
>> + * Rongrong Zou <zourongrong@huawei.com>
>> + * Rongrong Zou <zourongrong@gmail.com>
>> + * Jianhua Li <lijianhua@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + */
>> +
>> +#ifndef HIBMC_DRM_HW_H
>> +#define HIBMC_DRM_HW_H
>> +
>> +#define OFF 0
>> +#define ON 1
>> +#define DISABLE 0
>> +#define ENABLE 1
>
> These are not good names. I think you can probably hardcode the 0's
> and 1's in the code instead of these. However, if you want to use
> them, at least add a HIBMC_ prefix
I like hardcode here, thanks.
>
>> +
>> +/* register definition */
>> +#define HIBMC_MISC_CTRL 0x4
>> +
>> +#define HIBMC_MSCCTL_LOCALMEM_RESET(x) ((x) << 6)
>> +#define HIBMC_MSCCTL_LOCALMEM_RESET_MASK 0x40
>> +
>> +#define RESET 0
>> +#define NORMAL 1
>
> Same naming nit here. Add a prefix
ditto.
>
>> +
>> +#define HIBMC_CURRENT_GATE 0x000040
>> +#define HIBMC_CURR_GATE_DISPLAY(x) ((x) << 2)
>> +#define HIBMC_CURR_GATE_DISPLAY_MASK 0x4
>> +
>> +#define HIBMC_CURR_GATE_LOCALMEM(x) ((x) << 1)
>> +#define HIBMC_CURR_GATE_LOCALMEM_MASK 0x2
>> +
>> +#define HIBMC_MODE0_GATE 0x000044
>> +#define HIBMC_MODE1_GATE 0x000048
>> +#define HIBMC_POWER_MODE_CTRL 0x00004C
>> +
>> +#define HIBMC_PW_MODE_CTL_OSC_INPUT(x) ((x) << 3)
>> +#define HIBMC_PW_MODE_CTL_OSC_INPUT_MASK 0x8
>> +
>> +#define HIBMC_PW_MODE_CTL_MODE(x) ((x) << 0)
>> +#define HIBMC_PW_MODE_CTL_MODE_MASK 0x03
>> +#define HIBMC_PW_MODE_CTL_MODE_SHIFT 0
>> +
>> +#define HIBMC_PW_MODE_CTL_MODE_MODE0 0
>> +#define HIBMC_PW_MODE_CTL_MODE_MODE1 1
>> +#define HIBMC_PW_MODE_CTL_MODE_SLEEP 2
>> +
>> +#define HIBMC_PANEL_PLL_CTRL 0x00005C
>> +#define HIBMC_CRT_PLL_CTRL 0x000060
>> +
>> +#define HIBMC_PLL_CTRL_BYPASS(x) ((x) << 18)
>> +#define HIBMC_PLL_CTRL_BYPASS_MASK 0x40000
>> +
>> +#define HIBMC_PLL_CTRL_POWER(x) ((x) << 17)
>> +#define HIBMC_PLL_CTRL_POWER_MASK 0x20000
>> +
>> +#define HIBMC_PLL_CTRL_INPUT(x) ((x) << 16)
>> +#define HIBMC_PLL_CTRL_INPUT_MASK 0x10000
>> +
>> +#define OSC 0
>
> Naming
ditto.
>
>> +#define TESTCLK 1
>
> This doesn't seem to be used?
will remove it in next version.
>
>> +
>> +#define HIBMC_PLL_CTRL_POD(x) ((x) << 14)
>> +#define HIBMC_PLL_CTRL_POD_MASK 0xC000
>> +
>> +#define HIBMC_PLL_CTRL_OD(x) ((x) << 12)
>> +#define HIBMC_PLL_CTRL_OD_MASK 0x3000
>> +
>> +#define HIBMC_PLL_CTRL_N(x) ((x) << 8)
>> +#define HIBMC_PLL_CTRL_N_MASK 0xF00
>> +
>> +#define HIBMC_PLL_CTRL_M(x) ((x) << 0)
>> +#define HIBMC_PLL_CTRL_M_MASK 0xFF
>> +
>> +#define HIBMC_CRT_DISP_CTL 0x80200
>> +
>> +#define HIBMC_CRT_DISP_CTL_CRTSELECT(x) ((x) << 25)
>> +#define HIBMC_CRT_DISP_CTL_CRTSELECT_MASK 0x2000000
>> +
>> +#define CRTSELECT_VGA 0
>> +#define CRTSELECT_CRT 1
>> +
>> +#define HIBMC_CRT_DISP_CTL_CLOCK_PHASE(x) ((x) << 14)
>> +#define HIBMC_CRT_DISP_CTL_CLOCK_PHASE_MASK 0x4000
>> +
>> +#define PHASE_ACTIVE_HIGH 0
>> +#define PHASE_ACTIVE_LOW 1
>> +
>> +#define HIBMC_CRT_DISP_CTL_VSYNC_PHASE(x) ((x) << 13)
>> +#define HIBMC_CRT_DISP_CTL_VSYNC_PHASE_MASK 0x2000
>> +
>> +#define HIBMC_CRT_DISP_CTL_HSYNC_PHASE(x) ((x) << 12)
>> +#define HIBMC_CRT_DISP_CTL_HSYNC_PHASE_MASK 0x1000
>> +
>> +#define HIBMC_CRT_DISP_CTL_TIMING(x) ((x) << 8)
>> +#define HIBMC_CRT_DISP_CTL_TIMING_MASK 0x100
>> +
>> +#define HIBMC_CRT_DISP_CTL_PLANE(x) ((x) << 2)
>> +#define HIBMC_CRT_DISP_CTL_PLANE_MASK 4
>> +
>> +#define HIBMC_CRT_DISP_CTL_FORMAT(x) ((x) << 0)
>> +#define HIBMC_CRT_DISP_CTL_FORMAT_MASK 0x03
>> +
>> +#define HIBMC_CRT_FB_ADDRESS 0x080204
>> +
>> +#define HIBMC_CRT_FB_WIDTH 0x080208
>> +#define HIBMC_CRT_FB_WIDTH_WIDTH(x) ((x) << 16)
>> +#define HIBMC_CRT_FB_WIDTH_WIDTH_MASK 0x3FFF0000
>> +#define HIBMC_CRT_FB_WIDTH_OFFS(x) ((x) << 0)
>> +#define HIBMC_CRT_FB_WIDTH_OFFS_MASK 0x3FFF
>> +
>> +#define HIBMC_CRT_HORZ_TOTAL 0x08020C
>> +#define HIBMC_CRT_HORZ_TOTAL_TOTAL(x) ((x) << 16)
>> +#define HIBMC_CRT_HORZ_TOTAL_TOTAL_MASK 0xFFF0000
>> +
>> +#define HIBMC_CRT_HORZ_TOTAL_DISPLAY_END(x) ((x) << 0)
>> +#define HIBMC_CRT_HORZ_TOTAL_DISPLAY_END_MASK 0xFFF
>> +
>> +#define HIBMC_CRT_HORZ_SYNC 0x080210
>> +#define HIBMC_CRT_HORZ_SYNC_WIDTH(x) ((x) << 16)
>> +#define HIBMC_CRT_HORZ_SYNC_WIDTH_MASK 0xFF0000
>> +
>> +#define HIBMC_CRT_HORZ_SYNC_START(x) ((x) << 0)
>> +#define HIBMC_CRT_HORZ_SYNC_START_MASK 0xFFF
>> +
>> +#define HIBMC_CRT_VERT_TOTAL 0x080214
>> +#define HIBMC_CRT_VERT_TOTAL_TOTAL(x) ((x) << 16)
>> +#define HIBMC_CRT_VERT_TOTAL_TOTAL_MASK 0x7FFF0000
>> +
>> +#define HIBMC_CRT_VERT_TOTAL_DISPLAY_END(x) ((x) << 0)
>> +#define HIBMC_CRT_VERT_TOTAL_DISPLAY_END_MASK 0x7FF
>> +
>> +#define HIBMC_CRT_VERT_SYNC 0x080218
>> +#define HIBMC_CRT_VERT_SYNC_HEIGHT(x) ((x) << 16)
>> +#define HIBMC_CRT_VERT_SYNC_HEIGHT_MASK 0x3F0000
>> +
>> +#define HIBMC_CRT_VERT_SYNC_START(x) ((x) << 0)
>> +#define HIBMC_CRT_VERT_SYNC_START_MASK 0x7FF
>> +
>> +/* Auto Centering */
>> +#define HIBMC_CRT_AUTO_CENTERING_TL 0x080280
>> +#define HIBMC_CRT_AUTO_CENTERING_TL_TOP(x) ((x) << 16)
>> +#define HIBMC_CRT_AUTO_CENTERING_TL_TOP_MSK 0x7FF0000
>> +
>> +#define HIBMC_CRT_AUTO_CENTERING_TL_LEFT(x) ((x) << 0)
>> +#define HIBMC_CRT_AUTO_CENTERING_TL_LEFT_MSK 0x7FF
>> +
>> +#define HIBMC_CRT_AUTO_CENTERING_BR 0x080284
>> +#define HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM(x) ((x) << 16)
>> +#define HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM_MASK 0x7FF0000
>> +
>> +#define HIBMC_CRT_AUTO_CENTERING_BR_RIGHT(x) ((x) << 0)
>> +#define HIBMC_CRT_AUTO_CENTERING_BR_RIGHT_MASK 0x7FF
>> +
>> +/* register to control panel output */
>> +#define DISPLAY_CONTROL_HISILE 0x80288
>> +
>> +#define HIBMC_RAW_INTERRUPT 0x80290
>> +#define HIBMC_RAW_INTERRUPT_VBLANK(x) ((x) << 2)
>> +#define HIBMC_RAW_INTERRUPT_VBLANK_MASK 0x4
>> +
>> +#define HIBMC_RAW_INTERRUPT_EN 0x80298
>> +#define HIBMC_RAW_INTERRUPT_EN_VBLANK(x) ((x) << 2)
>> +#define HIBMC_RAW_INTERRUPT_EN_VBLANK_MASK 0x4
>> +
>> +/* register and values for PLL control */
>> +#define CRT_PLL1_HS 0x802a8
>> +#define CRT_PLL1_HS_25MHZ 0x23d40f02
>> +#define CRT_PLL1_HS_40MHZ 0x23940801
>> +#define CRT_PLL1_HS_65MHZ 0x23940d01
>> +#define CRT_PLL1_HS_78MHZ 0x23540F82
>> +#define CRT_PLL1_HS_74MHZ 0x23941dc2
>> +#define CRT_PLL1_HS_80MHZ 0x23941001
>> +#define CRT_PLL1_HS_80MHZ_1152 0x23540fc2
>> +#define CRT_PLL1_HS_108MHZ 0x23b41b01
>> +#define CRT_PLL1_HS_162MHZ 0x23480681
>> +#define CRT_PLL1_HS_148MHZ 0x23541dc2
>> +#define CRT_PLL1_HS_193MHZ 0x234807c1
>> +
>> +#define CRT_PLL2_HS 0x802ac
>> +#define CRT_PLL2_HS_25MHZ 0x206B851E
>> +#define CRT_PLL2_HS_40MHZ 0x30000000
>> +#define CRT_PLL2_HS_65MHZ 0x40000000
>> +#define CRT_PLL2_HS_78MHZ 0x50E147AE
>> +#define CRT_PLL2_HS_74MHZ 0x602B6AE7
>> +#define CRT_PLL2_HS_80MHZ 0x70000000
>> +#define CRT_PLL2_HS_108MHZ 0x80000000
>> +#define CRT_PLL2_HS_162MHZ 0xA0000000
>> +#define CRT_PLL2_HS_148MHZ 0xB0CCCCCD
>> +#define CRT_PLL2_HS_193MHZ 0xC0872B02
>> +
>> +/* Global macros */
>> +#define RGB(r, g, b) \
>
> Not used anywhere?
will remove it in next version.
>
>> +( \
>> + (unsigned long)(((r) << 16) | ((g) << 8) | (b)) \
>> +)
>> +
>> +#define PADDING(align, data) (((data) + (align) - 1) & (~((align) - 1)))
>> +
>
> This is only used in hibmc_drm_de.c, move it in there. It might also
> be nice to make it a type-checked function while you're at it.
agreed, thanks.
>
>
>> +#define MB(x) ((x) << 20)
>> +
>
> This is only used in 2 places, I think you can just hardcode the value
> in their respective #defines
agreed, thanks.
Regards,
Rongrong.
>
>> +#endif
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linuxarm mailing list
> linuxarm at huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
>
> .
>
^ permalink raw reply
* [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
From: Chen-Yu Tsai @ 2016-11-11 3:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_Jsq+Bw4XVEJY6V4Hn+D_OmqEcp_nLLrVT0_i+jX_guCqzfA@mail.gmail.com>
On Fri, Nov 11, 2016 at 5:42 AM, Rob Herring <robh@kernel.org> wrote:
> On Sun, Nov 6, 2016 at 7:56 PM, Chen-Yu Tsai <wens@csie.org> wrote:
>> On Mon, Nov 7, 2016 at 9:29 AM, Peter Chen <hzpeterchen@gmail.com> wrote:
>>> On Fri, Nov 04, 2016 at 01:51:34PM -0700, Stephen Boyd wrote:
>>>> Quoting Peter Chen (2016-10-24 18:16:32)
>>>> > On Mon, Oct 24, 2016 at 12:48:24PM -0700, Stephen Boyd wrote:
>>>> > > Quoting Chen-Yu Tsai (2016-10-24 05:19:05)
>>>> > > > Hi,
>>>> > > >
>>>> > > > On Tue, Oct 18, 2016 at 9:56 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
>>>> > > > > The ULPI bus can be built as a module, and it will soon be
>>>> > > > > calling these functions when it supports probing devices from DT.
>>>> > > > > Export them so they can be used by the ULPI module.
>>>> > > > >
>>>> > > > > Acked-by: Rob Herring <robh@kernel.org>
>>>> > > > > Cc: <devicetree@vger.kernel.org>
>>>> > > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
>>>> > > > > ---
>>>> > > > > drivers/of/device.c | 2 ++
>>>> > > > > 1 file changed, 2 insertions(+)
>>>> > > > >
>>>> > > > > diff --git a/drivers/of/device.c b/drivers/of/device.c
>>>> > > > > index 8a22a253a830..6719ab35b62e 100644
>>>> > > > > --- a/drivers/of/device.c
>>>> > > > > +++ b/drivers/of/device.c
>>>> > > > > @@ -225,6 +225,7 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
>>>> > > > >
>>>> > > > > return tsize;
>>>> > > > > }
>>>> > > > > +EXPORT_SYMBOL_GPL(of_device_get_modalias);
>>>> > > > >
>>>> > > > > int of_device_request_module(struct device *dev)
>>>> > > > > {
>>>> > > > > @@ -290,6 +291,7 @@ void of_device_uevent(struct device *dev, struct kobj_uevent_env *env)
>>>> > > > > }
>>>> > > > > mutex_unlock(&of_mutex);
>>>> > > > > }
>>>> > > > > +EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
>>>> > > >
>>>> > > > This is trailing the wrong function.
>>>> > > >
>>>> > >
>>>> > > Good catch. Must have been some bad rebase.
>>>> > >
>>>> > > Peter, can you fix it while applying or should I resend this patch?
>>>> > >
>>>> >
>>>> > But, this is device tree patch. I can only get chipidea part and other
>>>> > USB patches if Greg agrees.
>>>> >
>>>>
>>>> Were you expecting Rob to take the drivers/of/* patches? Sorry I thought
>>>> Rob acked them so they could go through usb with the other changes.
>>>
>>> I am just worried about possible merge error when linus pulls both OF
>>> and USB tree. Greg, is it ok the OF patches through USB tree with OF
>>> maintainer's ack?
>>
>> May I suggest putting the OF patches on an immutable branch so other
>> subsystems can pull them in without pulling in the USB patches? At
>> least I want to use them in the I2C subsystem, and in the sunxi-rsb
>> driver.
>
> Do you have patches using this already. If not, it is starting to get
> a bit late for v4.10.
>
> I can apply this, but then you'll just be pulling in other DT patches.
Not sure what you mean by "using" this...
I have patches which use this to add DT-based modalias entries for
module auto-loading to i2c and sunxi-rsb that I haven't sent.
As far as DT usage goes, we already need this for the axp20x mfd driver.
There are 2 variants, i2c and sunxi-rsb. For the I2C variant a fix was
sent to fix module auto-loading by using the I2C client ID table:
mfd: axp20x-i2c: Add i2c-ids to fix module auto-loading
https://git.kernel.org/cgit/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=b7142a19321484bd7681aa547c1d50148c8e2825
But for the sunxi-rsb variant such a fix does not exist, as the bus
does not have a separate ID table. It uses DT exclusively.
Regards
ChenYu
^ permalink raw reply
* [PATCH v27 1/9] memblock: add memblock_cap_memory_range()
From: AKASHI Takahiro @ 2016-11-11 2:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161110172720.GB17134@arm.com>
Will,
(+ Cc: Dennis)
On Thu, Nov 10, 2016 at 05:27:20PM +0000, Will Deacon wrote:
> On Wed, Nov 02, 2016 at 01:51:53PM +0900, AKASHI Takahiro wrote:
> > Add memblock_cap_memory_range() which will remove all the memblock regions
> > except the range specified in the arguments.
> >
> > This function, like memblock_mem_limit_remove_map(), will not remove
> > memblocks with MEMMAP_NOMAP attribute as they may be mapped and accessed
> > later as "device memory."
> > See the commit a571d4eb55d8 ("mm/memblock.c: add new infrastructure to
> > address the mem limit issue").
> >
> > This function is used, in a succeeding patch in the series of arm64 kdump
> > suuport, to limit the range of usable memory, System RAM, on crash dump
> > kernel.
> > (Please note that "mem=" parameter is of little use for this purpose.)
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Cc: linux-mm at kvack.org
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > include/linux/memblock.h | 1 +
> > mm/memblock.c | 28 ++++++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 5b759c9..0e770af 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -334,6 +334,7 @@ phys_addr_t memblock_start_of_DRAM(void);
> > phys_addr_t memblock_end_of_DRAM(void);
> > void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > void memblock_mem_limit_remove_map(phys_addr_t limit);
> > +void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > bool memblock_is_memory(phys_addr_t addr);
> > int memblock_is_map_memory(phys_addr_t addr);
> > int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 7608bc3..eb53876 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -1544,6 +1544,34 @@ void __init memblock_mem_limit_remove_map(phys_addr_t limit)
> > (phys_addr_t)ULLONG_MAX);
> > }
> >
> > +void __init memblock_cap_memory_range(phys_addr_t base, phys_addr_t size)
> > +{
> > + int start_rgn, end_rgn;
> > + int i, ret;
> > +
> > + if (!size)
> > + return;
> > +
> > + ret = memblock_isolate_range(&memblock.memory, base, size,
> > + &start_rgn, &end_rgn);
> > + if (ret)
> > + return;
> > +
> > + /* remove all the MAP regions */
> > + for (i = memblock.memory.cnt - 1; i >= end_rgn; i--)
> > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > + memblock_remove_region(&memblock.memory, i);
> > +
> > + for (i = start_rgn - 1; i >= 0; i--)
> > + if (!memblock_is_nomap(&memblock.memory.regions[i]))
> > + memblock_remove_region(&memblock.memory, i);
> > +
> > + /* truncate the reserved regions */
> > + memblock_remove_range(&memblock.reserved, 0, base);
> > + memblock_remove_range(&memblock.reserved,
> > + base + size, (phys_addr_t)ULLONG_MAX);
> > +}
>
> This duplicates a bunch of the logic in memblock_mem_limit_remove_map. Can
> you not implement that in terms of your new, more general, function? e.g.
> by passing base == 0, and size == limit?
Obviously it's possible.
I actually talked to Dennis before about merging them,
but he was against my idea.
Thanks,
-Takahiro AKASHI
> Will
^ permalink raw reply
* [PATCH v2 3/3] pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
The sunxi_pconf_reg helper introduced in the last patch gives us the
chance to rework sunxi_pconf_group_set to have it match the structure
of sunxi_pconf_(group_)get and make it easier to understand.
For each config to set, it:
1. checks if the parameter is supported.
2. checks if the argument is within limits.
3. converts argument to the register value.
4. writes to the register with spinlock held.
As a result the function now blocks unsupported config parameters,
instead of silently ignoring them.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 66 +++++++++++++++++------------------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 3e9f7c675d36..fa11a3100346 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -532,23 +532,27 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
struct sunxi_pinctrl_group *g = &pctl->groups[group];
- unsigned long flags;
unsigned pin = g->pin - pctl->desc->pin_base;
- u32 val, mask;
- u16 strength;
- u8 dlevel;
int i;
- spin_lock_irqsave(&pctl->lock, flags);
-
for (i = 0; i < num_configs; i++) {
- switch (pinconf_to_config_param(configs[i])) {
+ enum pin_config_param param;
+ unsigned long flags;
+ u32 offset, shift, mask, reg;
+ u16 arg, val;
+ int ret;
+
+ param = pinconf_to_config_param(configs[i]);
+ arg = pinconf_to_config_argument(configs[i]);
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ switch (param) {
case PIN_CONFIG_DRIVE_STRENGTH:
- strength = pinconf_to_config_argument(configs[i]);
- if (strength > 40) {
- spin_unlock_irqrestore(&pctl->lock, flags);
+ if (arg < 10 || arg > 40)
return -EINVAL;
- }
/*
* We convert from mA to what the register expects:
* 0: 10mA
@@ -556,39 +560,33 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
* 2: 30mA
* 3: 40mA
*/
- dlevel = strength / 10 - 1;
- val = readl(pctl->membase + sunxi_dlevel_reg(pin));
- mask = DLEVEL_PINS_MASK << sunxi_dlevel_offset(pin);
- writel((val & ~mask)
- | dlevel << sunxi_dlevel_offset(pin),
- pctl->membase + sunxi_dlevel_reg(pin));
+ val = arg / 10 - 1;
break;
case PIN_CONFIG_BIAS_DISABLE:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask),
- pctl->membase + sunxi_pull_reg(pin));
+ val = 0;
break;
case PIN_CONFIG_BIAS_PULL_UP:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 1 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 1;
break;
case PIN_CONFIG_BIAS_PULL_DOWN:
- val = readl(pctl->membase + sunxi_pull_reg(pin));
- mask = PULL_PINS_MASK << sunxi_pull_offset(pin);
- writel((val & ~mask) | 2 << sunxi_pull_offset(pin),
- pctl->membase + sunxi_pull_reg(pin));
+ if (arg == 0)
+ return -EINVAL;
+ val = 2;
break;
default:
- break;
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
}
- /* cache the config value */
- g->config = configs[i];
- } /* for each config */
- spin_unlock_irqrestore(&pctl->lock, flags);
+ spin_lock_irqsave(&pctl->lock, flags);
+ reg = readl(pctl->membase + offset);
+ reg &= ~(mask << shift);
+ writel(reg | val << shift, pctl->membase + offset);
+ spin_unlock_irqrestore(&pctl->lock, flags);
+ } /* for each config */
return 0;
}
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
index 0afce1ab12d0..a7efb31d6523 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
@@ -109,7 +109,6 @@ struct sunxi_pinctrl_function {
struct sunxi_pinctrl_group {
const char *name;
- unsigned long config;
unsigned pin;
};
--
2.10.2
^ permalink raw reply related
* [PATCH v2 2/3] pinctrl: sunxi: Add support for fetching pinconf settings from hardware
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
The sunxi pinctrl driver only caches whatever pinconf setting was last
set on a given pingroup. This is not particularly helpful, nor is it
correct.
Fix this by actually reading the hardware registers and returning
the correct results or error codes. Also filter out unsupported
pinconf settings. Since this driver has a peculiar setup of 1 pin
per group, we can support both pin and pingroup pinconf setting
read back with the same code. The sunxi_pconf_reg helper and code
structure is inspired by pinctrl-msm.
With this done we can also claim to support generic pinconf, by
setting .is_generic = true in pinconf_ops.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 84 +++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e04edda8629d..3e9f7c675d36 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -438,15 +438,91 @@ static const struct pinctrl_ops sunxi_pctrl_ops = {
.get_group_pins = sunxi_pctrl_get_group_pins,
};
+static int sunxi_pconf_reg(unsigned pin, enum pin_config_param param,
+ u32 *offset, u32 *shift, u32 *mask)
+{
+ switch (param) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ *offset = sunxi_dlevel_reg(pin);
+ *shift = sunxi_dlevel_offset(pin);
+ *mask = DLEVEL_PINS_MASK;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ case PIN_CONFIG_BIAS_DISABLE:
+ *offset = sunxi_pull_reg(pin);
+ *shift = sunxi_pull_offset(pin);
+ *mask = PULL_PINS_MASK;
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+
+ return 0;
+}
+
+static int sunxi_pconf_get(struct pinctrl_dev *pctldev, unsigned pin,
+ unsigned long *config)
+{
+ struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ u32 offset, shift, mask, val;
+ u16 arg;
+ int ret;
+
+ pin -= pctl->desc->pin_base;
+
+ ret = sunxi_pconf_reg(pin, param, &offset, &shift, &mask);
+ if (ret < 0)
+ return ret;
+
+ val = (readl(pctl->membase + offset) >> shift) & mask;
+
+ switch (pinconf_to_config_param(*config)) {
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ arg = (val + 1) * 10;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_UP:
+ if (val != SUN4I_PINCTRL_PULL_UP)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-up */
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (val != SUN4I_PINCTRL_PULL_DOWN)
+ return -EINVAL;
+ arg = 1; /* hardware is weak pull-down */
+ break;
+
+ case PIN_CONFIG_BIAS_DISABLE:
+ if (val != SUN4I_PINCTRL_NO_PULL)
+ return -EINVAL;
+ arg = 0;
+ break;
+
+ default:
+ /* sunxi_pconf_reg should catch anything unsupported */
+ WARN_ON(1);
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+
+ return 0;
+}
+
static int sunxi_pconf_group_get(struct pinctrl_dev *pctldev,
unsigned group,
unsigned long *config)
{
struct sunxi_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ struct sunxi_pinctrl_group *g = &pctl->groups[group];
- *config = pctl->groups[group].config;
-
- return 0;
+ /* We only support 1 pin per group. Chain it to the pin callback */
+ return sunxi_pconf_get(pctldev, g->pin, config);
}
static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
@@ -518,6 +594,8 @@ static int sunxi_pconf_group_set(struct pinctrl_dev *pctldev,
}
static const struct pinconf_ops sunxi_pconf_ops = {
+ .is_generic = true,
+ .pin_config_get = sunxi_pconf_get,
.pin_config_group_get = sunxi_pconf_group_get,
.pin_config_group_set = sunxi_pconf_group_set,
};
--
2.10.2
^ permalink raw reply related
* [PATCH v2 1/3] pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN, UP} argument
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161111024455.16883-1-wens@csie.org>
According to pinconf-generic.h, the argument for
PIN_CONFIG_BIAS_PULL_{DOWN,UP} is non-zero if the bias is enabled
with a pull up/down resistor, zero if it is directly connected
to VDD or ground.
Since Allwinner hardware uses a weak pull resistor internally,
the argument should be 1.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index e199d95af8c0..e04edda8629d 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -291,12 +291,16 @@ static unsigned long *sunxi_pctrl_build_pin_config(struct device_node *node,
if (sunxi_pctrl_has_bias_prop(node)) {
int pull = sunxi_pctrl_parse_bias_prop(node);
+ int arg = 0;
if (pull < 0) {
ret = pull;
goto err_free;
}
- pinconfig[idx++] = pinconf_to_config_packed(pull, 0);
+ if (pull != PIN_CONFIG_BIAS_DISABLE)
+ arg = 1; /* hardware uses weak pull resistors */
+
+ pinconfig[idx++] = pinconf_to_config_packed(pull, arg);
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 0/3] pinctrl: sunxi: Support generic pinconf functions
From: Chen-Yu Tsai @ 2016-11-11 2:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi everyone,
This series fixes up generic pinconf support for the sunxi pinctrl driver
library. The driver was doing some bits wrong, like a) storing the pinconf
config value in its struct, and not actually reading the hardware to get
the current config, and b) not using the right arguments for the bias
parameters.
Patch 1 fixes the pin bias parameter arguments.
Patch 2 makes the driver read out pinconf settings from the hardware, and
returns the correct value for unsupported features and disable features.
With this in place it also declares itself as generic pinconf compatible,
which enables us to read the config through the debugfs pinconf interface.
Patch 3 makes the sunxi_pconf_group_set callback use the helper function
introduced in patch 1.
Changes since v1:
- Rebased onto the updated sunxi pinctrl driver with support for the
generic pinconf bindings
- Use separate value for what is written to the register in the pinconf
set function, as Maxime requested.
Regards
ChenYu
Chen-Yu Tsai (3):
pinctrl: sunxi: Fix PIN_CONFIG_BIAS_PULL_{DOWN,UP} argument
pinctrl: sunxi: Add support for fetching pinconf settings from
hardware
pinctrl: sunxi: Make sunxi_pconf_group_set use sunxi_pconf_reg helper
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 156 +++++++++++++++++++++++++---------
drivers/pinctrl/sunxi/pinctrl-sunxi.h | 1 -
2 files changed, 118 insertions(+), 39 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH] pinctrl: sunxi: Free configs in pinctrl_map only if it is a config map
From: Chen-Yu Tsai @ 2016-11-11 2:35 UTC (permalink / raw)
To: linux-arm-kernel
In the recently refactored sunxi pinctrl library, we are only allocating
one set of pin configs for each pinmux setting node. When the pinctrl_map
structure is freed, the pin configs should also be freed. However the
code assumed the first map would contain the configs, which actually
never happens, as the mux function map gets added first.
The proper way to do this is to look through all the maps and free the
first one whose type is actually PIN_MAP_TYPE_CONFIGS_GROUP.
Also slightly expand the comment explaining this.
Fixes: f233dbca6227 ("pinctrl: sunxi: Rework the pin config building code")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index ebe2c73d211e..e199d95af8c0 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -408,8 +408,21 @@ static void sunxi_pctrl_dt_free_map(struct pinctrl_dev *pctldev,
struct pinctrl_map *map,
unsigned num_maps)
{
- /* All the maps have the same pin config, free only the first one */
- kfree(map[0].data.configs.configs);
+ int i;
+
+ /* pin config is never in the first map */
+ for (i = 1; i < num_maps; i++) {
+ if (map[i].type != PIN_MAP_TYPE_CONFIGS_GROUP)
+ continue;
+
+ /*
+ * All the maps share the same pin config,
+ * free only the first one we find.
+ */
+ kfree(map[i].data.configs.configs);
+ break;
+ }
+
kfree(map);
}
--
2.10.2
^ permalink raw reply related
* [PATCH v9 2/8] power: add power sequence library
From: Peter Chen @ 2016-11-11 1:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJZ5v0ioqMqoaJqK0CTXafWRKoohmZSiOWKRgVZ9DR3_8+-YDw@mail.gmail.com>
On Fri, Nov 11, 2016 at 02:05:01AM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 8, 2016 at 3:51 AM, Peter Chen <peter.chen@nxp.com> wrote:
> > We have an well-known problem that the device needs to do some power
> > sequence before it can be recognized by related host, the typical
> > example like hard-wired mmc devices and usb devices.
> >
> > This power sequence is hard to be described at device tree and handled by
> > related host driver, so we have created a common power sequence
> > library to cover this requirement. The core code has supplied
> > some common helpers for host driver, and individual power sequence
> > libraries handle kinds of power sequence for devices. The pwrseq
> > librares always need to allocate extra instance for compatible
> > string match.
> >
> > pwrseq_generic is intended for general purpose of power sequence, which
> > handles gpios and clocks currently, and can cover other controls in
> > future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence is needed, else call of_pwrseq_on_list
> > /of_pwrseq_off_list instead (eg, USB hub driver).
> >
> > For new power sequence library, it can add its compatible string
> > to pwrseq_of_match_table, then the pwrseq core will match it with
> > DT's, and choose this library at runtime.
>
> In the first place, please document this stuff better than you have so
> far. To a minimum, add kerneldoc comments to all new non-trivial new
> functions to document what they are for and how they are expected to
> be used (especially the ones exported to drivers).
>
Thanks for your comments.
I will add kerneldoc for main APIs.
> Also, is there any guidance available for people who may want to use it?
No doc now, only some guidance in this commit log.
> > +config PWRSEQ_GENERIC
> > + bool "Generic power sequence control"
> > + depends on OF
> > + select POWER_SEQUENCE
> > + help
> > + It is used for drivers which needs to do power sequence
> > + (eg, turn on clock, toggle reset gpio) before the related
> > + devices can be found by hardware. This generic one can be
> > + used for common power sequence control.
>
> I wouldn't set it up this way.
>
> There are two problems here.
>
> First, say a distro is going to ship a multiplatform generic kernel.
> How they are going to figure out whether or not to set the new symbol
> in that kernel?
>
> Second, how users are supposed to know whether or not they will need
> it even if they build the kernel by themselves?
>
> It would be better IMO to set things up to select the new symbol from
> places making use of the code depending on it.
>
Will change it like below:
#
# Power Sequence library
#
menuconfig POWER_SEQUENCE
bool "Power sequence control"
depends on OF
help
It is used for drivers which needs to do power sequence
(eg, turn on clock, toggle reset gpio) before the related
devices can be found by hardware.
if POWER_SEQUENCE
config PWRSEQ_GENERIC
bool "Generic power sequence control"
default y
help
This is the generic power sequence control library, and is
supposed to support common power sequence usage.
endif
And the current user usb core will select POWER_SEQUENCE.
--
Best Regards,
Peter Chen
^ permalink raw reply
* [PATCH v6 9/9] MAINTAINERS: Update HISILICON DRM entries
From: Sean Paul @ 2016-11-11 1:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-10-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
Acked-by: Sean Paul <seanpaul@chromium.org>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c447953..cc5ee3a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4117,6 +4117,7 @@ F: drivers/gpu/drm/gma500/
>
> DRM DRIVERS FOR HISILICON
> M: Xinliang Liu <z.liuxinliang@hisilicon.com>
> +M: Rongrong Zou <zourongrong@gmail.com>
> R: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> R: Chen Feng <puck.chen@hisilicon.com>
> L: dri-devel at lists.freedesktop.org
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 8/9] drm/hisilicon/hibmc: Add vblank interruput
From: Sean Paul @ 2016-11-11 1:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-9-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add vblank interrupt.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 56 ++++++++++++++++++++++++-
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 1 +
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 4253603..b668e3e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -40,16 +40,46 @@
>
> static int hibmc_enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> +
> + writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(1),
> + hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
> +
> return 0;
> }
>
> static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> +
> + writel(HIBMC_RAW_INTERRUPT_EN_VBLANK(0),
> + hidev->mmio + HIBMC_RAW_INTERRUPT_EN);
> +}
> +
> +irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
> +{
> + struct drm_device *dev = (struct drm_device *)arg;
> + struct hibmc_drm_device *hidev =
> + (struct hibmc_drm_device *)dev->dev_private;
> + struct drm_crtc *crtc = &hidev->crtc;
> + u32 status;
> +
> + status = readl(hidev->mmio + HIBMC_RAW_INTERRUPT);
> +
> + if (status & HIBMC_RAW_INTERRUPT_VBLANK(1)) {
> + writel(HIBMC_RAW_INTERRUPT_VBLANK(1),
> + hidev->mmio + HIBMC_RAW_INTERRUPT);
> + drm_crtc_handle_vblank(crtc);
> + }
> +
> + return IRQ_HANDLED;
> }
>
> static struct drm_driver hibmc_driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET |
> - DRIVER_ATOMIC,
> + DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
> .fops = &hibmc_fops,
> .name = "hibmc",
> .date = "20160828",
> @@ -63,6 +93,7 @@ static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> .dumb_create = hibmc_dumb_create,
> .dumb_map_offset = hibmc_dumb_mmap_offset,
> .dumb_destroy = drm_gem_dumb_destroy,
> + .irq_handler = hibmc_drm_interrupt,
> };
>
> static int hibmc_pm_suspend(struct device *dev)
> @@ -242,6 +273,13 @@ static int hibmc_unload(struct drm_device *dev)
> struct hibmc_drm_device *hidev = dev->dev_private;
>
> hibmc_fbdev_fini(hidev);
> +
> + if (dev->irq_enabled)
> + drm_irq_uninstall(dev);
> + if (hidev->msi_enabled)
> + pci_disable_msi(dev->pdev);
> + drm_vblank_cleanup(dev);
> +
> hibmc_kms_fini(hidev);
> hibmc_mm_fini(hidev);
> hibmc_hw_fini(hidev);
> @@ -272,6 +310,22 @@ static int hibmc_load(struct drm_device *dev, unsigned long flags)
> if (ret)
> goto err;
>
> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> + if (ret) {
> + DRM_ERROR("failed to initialize vblank.\n");
> + goto err;
> + }
> +
> + hidev->msi_enabled = 0;
> + if (pci_enable_msi(dev->pdev)) {
It would be useful to check and print the return value of this.
> + DRM_ERROR("Enabling MSI failed!\n");
> + } else {
> + hidev->msi_enabled = 1;
> + ret = drm_irq_install(dev, dev->pdev->irq);
> + if (ret)
> + DRM_ERROR("install irq failed , ret = %d\n", ret);
DRM_WARN might be more appropriate, given that this isn't considered fatal.
> + }
> +
> /* reset all the states of crtc/plane/encoder/connector */
> drm_mode_config_reset(dev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 450247d..f1706fb 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -42,6 +42,7 @@ struct hibmc_drm_device {
> void __iomem *fb_map;
> unsigned long fb_base;
> unsigned long fb_size;
> + int msi_enabled;
Why not bool?
>
> /* drm */
> struct drm_device *dev;
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 7/9] drm/hisilicon/hibmc: Add connector for VDAC
From: Sean Paul @ 2016-11-11 1:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477639682-22520-8-git-send-email-zourongrong@gmail.com>
On Fri, Oct 28, 2016 at 3:28 AM, Rongrong Zou <zourongrong@gmail.com> wrote:
> Add connector funcs and helper funcs for VDAC.
>
> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
> ---
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 8 +++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 76 ++++++++++++++++++++++++
> 3 files changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index ba191e1..4253603 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -131,6 +131,14 @@ static int hibmc_kms_init(struct hibmc_drm_device *hidev)
> return ret;
> }
>
> + ret = hibmc_connector_init(hidev);
> + if (ret) {
> + DRM_ERROR("failed to init connector\n");
> + return ret;
> + }
> +
> + drm_mode_connector_attach_encoder(&hidev->connector,
> + &hidev->encoder);
The connector should be initialized in the vdac driver with the
encoder, not in the drv file (same as plane/crtc)
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 401cea4..450247d 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -48,6 +48,7 @@ struct hibmc_drm_device {
> struct drm_plane plane;
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> + struct drm_connector connector;
No need to keep track here
> bool mode_config_initialized;
>
> /* ttm */
> @@ -89,6 +90,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct drm_gem_object *gem)
> int hibmc_plane_init(struct hibmc_drm_device *hidev);
> int hibmc_crtc_init(struct hibmc_drm_device *hidev);
> int hibmc_encoder_init(struct hibmc_drm_device *hidev);
> +int hibmc_connector_init(struct hibmc_drm_device *hidev);
> int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
> void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> index 953f659..ebefcd1 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
> @@ -87,3 +87,79 @@ int hibmc_encoder_init(struct hibmc_drm_device *hidev)
> drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
> return 0;
> }
> +
> +static int hibmc_connector_get_modes(struct drm_connector *connector)
> +{
> + int count;
> +
> + count = drm_add_modes_noedid(connector, 800, 600);
> + drm_set_preferred_mode(connector, defx, defy);
So you have defx/defy as module parameters, but then hardcode the
800x600 mode. If defx/defy is anything other than 800/600, this won't
work. I think you should just remove the defx/defy module params and
rely on userspace adding modes as appropriate.
> + return count;
> +}
> +
> +static int hibmc_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct hibmc_drm_device *hiprivate =
> + container_of(connector, struct hibmc_drm_device, connector);
> + unsigned long size = mode->hdisplay * mode->vdisplay * 4;
Why * 4 here and why * 2 below? You support formats less than 32 bpp,
so the * 4 isn't necessarily correct for all formats. Is the * 2 to
account for front & back buffer?
> +
> + if (size * 2 > hiprivate->fb_size)
> + return MODE_BAD;
> +
> + return MODE_OK;
> +}
> +
> +static struct drm_encoder *
> +hibmc_connector_best_encoder(struct drm_connector *connector)
> +{
> + int enc_id = connector->encoder_ids[0];
> +
> + /* pick the encoder ids */
> + if (enc_id)
> + return drm_encoder_find(connector->dev, enc_id);
Can't you just do return drm_encoder_find(connector->dev,
connector->encoder_ids[0]); ?
ie: won't drm_encoder_find do the right thing if you pass in id == 0?
> +
> + return NULL;
> +}
> +
> +static enum drm_connector_status hibmc_connector_detect(struct drm_connector
> + *connector, bool force)
> +{
> + return connector_status_connected;
Perhaps this should be connector_status_unknown, since you don't
necessarily know it's connected.
> +}
> +
> +static const struct drm_connector_helper_funcs
> + hibmc_connector_connector_helper_funcs = {
> + .get_modes = hibmc_connector_get_modes,
> + .mode_valid = hibmc_connector_mode_valid,
> + .best_encoder = hibmc_connector_best_encoder,
> +};
> +
> +static const struct drm_connector_funcs hibmc_connector_connector_funcs = {
> + .dpms = drm_atomic_helper_connector_dpms,
> + .detect = hibmc_connector_detect,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +int hibmc_connector_init(struct hibmc_drm_device *hidev)
> +{
> + struct drm_device *dev = hidev->dev;
> + struct drm_connector *connector = &hidev->connector;
> + int ret;
> +
> + ret = drm_connector_init(dev, connector,
> + &hibmc_connector_connector_funcs,
> + DRM_MODE_CONNECTOR_VGA);
> + if (ret) {
> + DRM_ERROR("failed to init connector\n");
> + return ret;
> + }
> + drm_connector_helper_add(connector,
> + &hibmc_connector_connector_helper_funcs);
> +
> + return 0;
> +}
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] drm/rockchip: return ERR_PTR instead of NULL
From: Mark yao @ 2016-11-11 1:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478812256-26189-1-git-send-email-Julia.Lawall@lip6.fr>
On 2016?11?11? 05:10, Julia Lawall wrote:
> rockchip_drm_framebuffer_init is only used in one case, in
> rockchip_drm_fbdev.c, where its return value is tested using IS_ERR. To
> enable propagating the reason for the error, change the definition so that
> it returns an ERR_PTR value.
>
> Problem found with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Thanks for the fix.
Applied to my drm-next.
>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 0f6eda0..01e11bf 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -213,7 +213,7 @@ struct drm_framebuffer *
>
> rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, &obj, 1);
> if (IS_ERR(rockchip_fb))
> - return NULL;
> + return ERR_CAST(rockchip_fb);
>
> return &rockchip_fb->fb;
> }
>
>
>
>
--
?ark Yao
^ permalink raw reply
* [PATCH v9 2/8] power: add power sequence library
From: Rafael J. Wysocki @ 2016-11-11 1:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478573472-29516-3-git-send-email-peter.chen@nxp.com>
On Tue, Nov 8, 2016 at 3:51 AM, Peter Chen <peter.chen@nxp.com> wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
>
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
>
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
>
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.
In the first place, please document this stuff better than you have so
far. To a minimum, add kerneldoc comments to all new non-trivial new
functions to document what they are for and how they are expected to
be used (especially the ones exported to drivers).
Also, is there any guidance available for people who may want to use it?
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> MAINTAINERS | 9 ++
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/pwrseq/Kconfig | 19 ++++
> drivers/power/pwrseq/Makefile | 2 +
> drivers/power/pwrseq/core.c | 191 ++++++++++++++++++++++++++++++++++
> drivers/power/pwrseq/pwrseq_generic.c | 183 ++++++++++++++++++++++++++++++++
> include/linux/power/pwrseq.h | 72 +++++++++++++
> 8 files changed, 478 insertions(+)
> create mode 100644 drivers/power/pwrseq/Kconfig
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/core.c
> create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
> create mode 100644 include/linux/power/pwrseq.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..5dab975 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9612,6 +9612,15 @@ F: include/linux/pm_*
> F: include/linux/powercap.h
> F: drivers/powercap/
>
> +POWER SEQUENCE LIBRARY
> +M: Peter Chen <Peter.Chen@nxp.com>
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
> +L: linux-pm at vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/power/pwrseq/
> +F: drivers/power/pwrseq/
> +F: include/linux/power/pwrseq.h/
> +
> POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
> M: Sebastian Reichel <sre@kernel.org>
> L: linux-pm at vger.kernel.org
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 63454b5..c1bb046 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -1,3 +1,4 @@
> source "drivers/power/avs/Kconfig"
> source "drivers/power/reset/Kconfig"
> source "drivers/power/supply/Kconfig"
> +source "drivers/power/pwrseq/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ff35c71..7db8035 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_POWER_AVS) += avs/
> obj-$(CONFIG_POWER_RESET) += reset/
> obj-$(CONFIG_POWER_SUPPLY) += supply/
> +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/
> diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
> new file mode 100644
> index 0000000..3859a67
> --- /dev/null
> +++ b/drivers/power/pwrseq/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Power Sequence library
> +#
> +
> +config POWER_SEQUENCE
> + bool
> +
> +menu "Power Sequence Support"
> +
> +config PWRSEQ_GENERIC
> + bool "Generic power sequence control"
> + depends on OF
> + select POWER_SEQUENCE
> + help
> + It is used for drivers which needs to do power sequence
> + (eg, turn on clock, toggle reset gpio) before the related
> + devices can be found by hardware. This generic one can be
> + used for common power sequence control.
I wouldn't set it up this way.
There are two problems here.
First, say a distro is going to ship a multiplatform generic kernel.
How they are going to figure out whether or not to set the new symbol
in that kernel?
Second, how users are supposed to know whether or not they will need
it even if they build the kernel by themselves?
It would be better IMO to set things up to select the new symbol from
places making use of the code depending on it.
Thanks,
Rafael
^ permalink raw reply
* [PATCH fpga 3/9] fpga zynq: Fix incorrect ISR state on bootup
From: Jason Gunthorpe @ 2016-11-11 0:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHeaaxf4L7aQ455Yrpm1fgqr=L-LxMd7yDQOKjLsBvp38w@mail.gmail.com>
On Thu, Nov 10, 2016 at 04:44:39PM -0800, Moritz Fischer wrote:
> > + zynq_fpga_write(priv, INT_MASK_OFFSET, 0xFFFFFFFF);
>
> Named constant please.
This line gets fixed in the next patch that, lets just leave it, less
churn..
> > + zynq_fpga_write(priv, INT_STS_OFFSET, IXR_ALL_MASK);
> > + err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, dev_name(dev),
> > + priv);
> > + if (err) {
> > + dev_err(dev, "unable to request IRQ\n");
> > + clk_unprepare(priv->clk);
>
> Your enable count is off in that case. This should be a clk_disable_unprepare()
> call.
Yep.
FWIW, it feels wrong to leave the ISR enabled but the clk
disabled..
Jason
^ permalink raw reply
* [PATCH fpga 3/9] fpga zynq: Fix incorrect ISR state on bootup
From: Moritz Fischer @ 2016-11-11 0:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478732303-13718-4-git-send-email-jgunthorpe@obsidianresearch.com>
Hi Jason,
On Wed, Nov 9, 2016 at 2:58 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> It is best practice to clear and mask all interrupts before
> associating the IRQ, and this should be done after the clock
> is enabled.
>
> This corrects a bad result from zynq_fpga_ops_state on bootup
> where left over latched values in INT_STS_OFFSET caused it to
> report an unconfigured FPGA as configured.
>
> After this change the boot up operating state for an unconfigured
> FPGA reports 'unknown'.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
> drivers/fpga/zynq-fpga.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
> index 86f4377e2b52..40cf0feaca7c 100644
> --- a/drivers/fpga/zynq-fpga.c
> +++ b/drivers/fpga/zynq-fpga.c
> @@ -460,13 +460,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
> return priv->irq;
> }
>
> - err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0,
> - dev_name(dev), priv);
> - if (err) {
> - dev_err(dev, "unable to request IRQ\n");
> - return err;
> - }
> -
> priv->clk = devm_clk_get(dev, "ref_clk");
> if (IS_ERR(priv->clk)) {
> dev_err(dev, "input clock not found\n");
> @@ -482,6 +475,16 @@ static int zynq_fpga_probe(struct platform_device *pdev)
> /* unlock the device */
> zynq_fpga_write(priv, UNLOCK_OFFSET, UNLOCK_MASK);
>
> + zynq_fpga_write(priv, INT_MASK_OFFSET, 0xFFFFFFFF);
Named constant please.
> + zynq_fpga_write(priv, INT_STS_OFFSET, IXR_ALL_MASK);
> + err = devm_request_irq(dev, priv->irq, zynq_fpga_isr, 0, dev_name(dev),
> + priv);
> + if (err) {
> + dev_err(dev, "unable to request IRQ\n");
> + clk_unprepare(priv->clk);
Your enable count is off in that case. This should be a clk_disable_unprepare()
call.
<snip>
clk_prepare_enable()
if(err) {
clk_unprepare();
return err; /* enable count = 1? <- huh? */
}
clk_disable() enable count = 0
clk_unprepare() prepare count = 0
</snip>
> + return err;
> + }
> +
> clk_disable(priv->clk);
>
> err = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager",
> --
> 2.1.4
>
Thanks,
Moritz
^ permalink raw reply
* [PATCH] arm: kprobe: replace patch_lock to raw lock
From: Yang Shi @ 2016-11-11 0:17 UTC (permalink / raw)
To: linux-arm-kernel
When running kprobe on -rt kernel, the below bug is caught:
BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:931
in_atomic(): 1, irqs_disabled(): 128, pid: 14, name: migration/0
INFO: lockdep is turned off.
irq event stamp: 238
hardirqs last enabled at (237): [<80b5aecc>] _raw_spin_unlock_irqrestore+0x88/0x90
hardirqs last disabled at (238): [<80b56d88>] __schedule+0xec/0x94c
softirqs last enabled at (0): [<80225584>] copy_process.part.5+0x30c/0x1994
softirqs last disabled at (0): [< (null)>] (null)
Preemption disabled at:[<802f2b98>] cpu_stopper_thread+0xc0/0x140
CPU: 0 PID: 14 Comm: migration/0 Tainted: G O 4.8.3-rt2 #1
Hardware name: Freescale LS1021A
[<80212e7c>] (unwind_backtrace) from [<8020cd2c>] (show_stack+0x20/0x24)
[<8020cd2c>] (show_stack) from [<80689e14>] (dump_stack+0xa0/0xcc)
[<80689e14>] (dump_stack) from [<8025a43c>] (___might_sleep+0x1b8/0x2a4)
[<8025a43c>] (___might_sleep) from [<80b5b324>] (rt_spin_lock+0x34/0x74)
[<80b5b324>] (rt_spin_lock) from [<80b5c31c>] (__patch_text_real+0x70/0xe8)
[<80b5c31c>] (__patch_text_real) from [<80b5c3ac>] (patch_text_stop_machine+0x18/0x20)
[<80b5c3ac>] (patch_text_stop_machine) from [<802f2920>] (multi_cpu_stop+0xfc/0x134)
[<802f2920>] (multi_cpu_stop) from [<802f2ba0>] (cpu_stopper_thread+0xc8/0x140)
[<802f2ba0>] (cpu_stopper_thread) from [<802563a4>] (smpboot_thread_fn+0x1a4/0x354)
[<802563a4>] (smpboot_thread_fn) from [<80251d38>] (kthread+0x104/0x11c)
[<80251d38>] (kthread) from [<80207f70>] (ret_from_fork+0x14/0x24)
Since patch_text_stop_machine() is called in stop_machine() which disables IRQ,
sleepable lock should be not used in this atomic context, so replace patch_lock
to raw lock.
Signed-off-by: Yang Shi <yang.shi@linaro.org>
---
arch/arm/kernel/patch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 69bda1a..1f665ac 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -15,7 +15,7 @@ struct patch {
unsigned int insn;
};
-static DEFINE_SPINLOCK(patch_lock);
+static DEFINE_RAW_SPINLOCK(patch_lock);
static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
__acquires(&patch_lock)
@@ -32,7 +32,7 @@ static void __kprobes *patch_map(void *addr, int fixmap, unsigned long *flags)
return addr;
if (flags)
- spin_lock_irqsave(&patch_lock, *flags);
+ raw_spin_lock_irqsave(&patch_lock, *flags);
else
__acquire(&patch_lock);
@@ -47,7 +47,7 @@ static void __kprobes patch_unmap(int fixmap, unsigned long *flags)
clear_fixmap(fixmap);
if (flags)
- spin_unlock_irqrestore(&patch_lock, *flags);
+ raw_spin_unlock_irqrestore(&patch_lock, *flags);
else
__release(&patch_lock);
}
--
2.0.2
^ permalink raw reply related
* [clk:clk-qcom-8994 2/2] include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
From: Stephen Boyd @ 2016-11-10 23:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201611110732.1ceuZvsr%fengguang.wu@intel.com>
On 11/11, kbuild test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-qcom-8994
> head: cc800227108710c8f02255e61659b956b041eec3
> commit: cc800227108710c8f02255e61659b956b041eec3 [2/2] clk: qcom: Add support for msm8994 global clock controller
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> git checkout cc800227108710c8f02255e61659b956b041eec3
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from drivers/clk/qcom/gcc-msm8994.c:20:0:
> >> include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
> extern const typeof(name) __mod_##type##__##name##_device_table \
> ^
> >> drivers/clk/qcom/gcc-msm8994.c:2265:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
> MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
Urgh that's sad. Too bad MODULE_DEVICE_TABLE doesn't have
something in it in the !MODULE case to cause compilation problems
like this to come out. I'll go fix this up.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v7] soc: qcom: add l2 cache perf events driver
From: Leeder, Neil @ 2016-11-10 23:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109175413.GE17020@leverpostej>
Hi Mark,
Thanks for the review. I'll handle all the syntactic comments, so I
won't reply to them all individually here.
For the aggregation, I'll reply separately to Will's post to
keep all those comments together.
On 11/9/2016 12:54 PM, Mark Rutland wrote:
>> +
>> +/*
>> + * The cache is made up of one or more clusters, each cluster has its own PMU.
>> + * Each cluster is associated with one or more CPUs.
>> + * This structure represents one of the hardware PMUs.
>> + *
>> + * Events can be envisioned as a 2-dimensional array. Each column represents
>> + * a group of events. There are 8 groups. Only one entry from each
>> + * group can be in use at a time. When an event is assigned a counter
>> + * by *_event_add(), the counter index is assigned to group_to_counter[group].
>
> If I've followed correctly, this means each group has a dedicated
> counter?
>
> I take it groups are not symmetric (i.e. each column has different
> events)? Or does each column contain the same events?
>
> Is there any overlap?
Each group will have at most one counter, but it's not dedicated.
There's no requirement that an implementation have as many counters as
there are groups, so an event can be assigned to any available counter.
Every entry in the 2-dimensional array is unique, so no duplicates. Once
you have used an event, you cannot use any other event from its column.
As an aside, hardware designers put in some effort to ensure that events
which may need to be collected at the same time are located in different
columns.
>> +static int l2_cache__event_init(struct perf_event *event)
>> +{
[...]
>> + /* Don't allow groups with mixed PMUs, except for s/w events */
>> + if (event->group_leader->pmu != event->pmu &&
>> + !is_software_event(event->group_leader)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create mixed PMU group\n");
>> + return -EINVAL;
>> + }
>> +
>> + list_for_each_entry(sibling, &event->group_leader->sibling_list,
>> + group_entry)
>> + if (sibling->pmu != event->pmu &&
>> + !is_software_event(sibling)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create mixed PMU group\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Ensure all events in a group are on the same cpu */
>> + cluster = get_hml2_pmu(event->cpu);
>> + if ((event->group_leader != event) &&
>> + (cluster->on_cpu != event->group_leader->cpu)) {
>> + dev_warn(&l2cache_pmu->pdev->dev,
>> + "Can't create group on CPUs %d and %d",
>> + event->cpu, event->group_leader->cpu);
>> + return -EINVAL;
>> + }
>
> It's probably worth also checking that the events are co-schedulable
> (e.g. they don't conflict column-wise).
That's what filter_match() is doing - stopping column-conflicting events
from even getting to init(). In init() we don't have a record of which
other events are being co-scheduled. We could keep a list of groups used
by other events to compare against, but because there's no matching
term() function there's no obvious way of removing them from the list.
In my very first patchset I had the check inside event_add() because
entries could be removed in event_del(). That was where you suggested
that I use filter_match().
>
>> + if (acpi_bus_get_device(ACPI_HANDLE(dev), &device))
>> + return -ENODEV;
>> +
>> + if (kstrtol(device->pnp.unique_id, 10, &fw_cluster_id) < 0) {
>> + dev_err(&pdev->dev, "unable to read ACPI uid\n");
>> + return -ENODEV;
>> + }
>
>> + cluster->l2cache_pmu = l2cache_pmu;
>> + for_each_present_cpu(cpu) {
>> + if (topology_physical_package_id(cpu) == fw_cluster_id) {
>> + cpumask_set_cpu(cpu, &cluster->cluster_cpus);
>> + per_cpu(pmu_cluster, cpu) = cluster;
>> + }
>> + }
>
> I'm still uneasy about this.
>
> The topology_* API doesn't have a well-defined mapping to the MPIDR.Aff*
> levels, which itself also don't have a well-defined mapping to your
> hardware's clusters (and therefore fw_cluster_id).
>
> Thus, I'm rather worried that this is going to get broken easily, either
> by changes in the kernel, or in future HW revisions where the mapping of
> clusters to MPIDR.Aff* fields changes.
I'm not sure how else to get a mapping of CPU to cluster which doesn't
eventually end with MPIDR.
This is the definition of topology_physical_package_id() from
asm/topology.h:
#define topology_physical_package_id(cpu)
(cpu_topology[cpu].cluster_id)
It seems to be a pretty solid connection between cpu and cluster. I
don't think this is an abuse of this function. Unless there is some
other way of getting this mapping I'd suggest using this, and if some
future chip should change MPIDR usage it can be addressed it then.
>
> Thanks,
> Mark.
>
Thanks,
Neil
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [clk:clk-qcom-8994 2/2] include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
From: kbuild test robot @ 2016-11-10 23:20 UTC (permalink / raw)
To: linux-arm-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-qcom-8994
head: cc800227108710c8f02255e61659b956b041eec3
commit: cc800227108710c8f02255e61659b956b041eec3 [2/2] clk: qcom: Add support for msm8994 global clock controller
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout cc800227108710c8f02255e61659b956b041eec3
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/clk/qcom/gcc-msm8994.c:20:0:
>> include/linux/module.h:213:1: error: expected ',' or ';' before 'extern'
extern const typeof(name) __mod_##type##__##name##_device_table \
^
>> drivers/clk/qcom/gcc-msm8994.c:2265:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
MODULE_DEVICE_TABLE(of, gcc_msm8994_match_table);
^~~~~~~~~~~~~~~~~~~
vim +213 include/linux/module.h
^1da177e Linus Torvalds 2005-04-16 207 /* What your module does. */
^1da177e Linus Torvalds 2005-04-16 208 #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
^1da177e Linus Torvalds 2005-04-16 209
cff26a51 Rusty Russell 2014-02-03 210 #ifdef MODULE
cff26a51 Rusty Russell 2014-02-03 211 /* Creates an alias so file2alias.c can find device table. */
^1da177e Linus Torvalds 2005-04-16 212 #define MODULE_DEVICE_TABLE(type, name) \
6301939d Andrey Ryabinin 2015-02-13 @213 extern const typeof(name) __mod_##type##__##name##_device_table \
cff26a51 Rusty Russell 2014-02-03 214 __attribute__ ((unused, alias(__stringify(name))))
cff26a51 Rusty Russell 2014-02-03 215 #else /* !MODULE */
cff26a51 Rusty Russell 2014-02-03 216 #define MODULE_DEVICE_TABLE(type, name)
:::::: The code at line 213 was first introduced by commit
:::::: 6301939d97d079f0d3dbe71e750f4daf5d39fc33 module: fix types of device tables aliases
:::::: TO: Andrey Ryabinin <a.ryabinin@samsung.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 56819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161111/9bf381ab/attachment-0001.gz>
^ permalink raw reply
* [Linaro-acpi] ACPI namespace details for ARM64
From: Al Stone @ 2016-11-10 23:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109220506.GN14322@bhelgaas-glaptop.roam.corp.google.com>
On 11/09/2016 03:05 PM, Bjorn Helgaas wrote:
> Hi all,
>
> We've been working through the details of getting ACPI to work on
> arm64, and there have been lots of questions about what this means for
> PCI. I've outlined this for several people individually, but I'm
> going to send this separately, apart from a specific patch series, to
> make sure we're all on the same page. Please correct my errors and
> misunderstandings.
>
> Bjorn
>
>[snip....]
A big +1 to all of this. This also looks like something that should
be added to either PCI, ACPI or arm64 documentation (or even all three).
What do you think?
Thank you for putting this together, Bjorn.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------
^ permalink raw reply
* [PATCH] staging: vc04_services: add vchiq_pagelist_info structure
From: Greg KH @ 2016-11-10 22:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478533287.17065.1.camel@crowfest.net>
On Mon, Nov 07, 2016 at 07:41:27AM -0800, Michael Zoran wrote:
> On Mon, 2016-11-07 at 11:03 +0100, Greg KH wrote:
> > On Mon, Oct 31, 2016 at 01:10:35AM -0700, Michael Zoran wrote:
> > > The current dma_map_sg based implementation for bulk messages
> > > computes many offsets into a single allocation multiple times in
> > > both the create and free code paths.??This is inefficient,
> > > error prone and in fact still has a few lingering issues
> > > with arm64.
> > >
> > > This change replaces a small portion of that inplementation with
> > > new code that uses a new struct vchiq_pagelist_info to store the
> > > needed information rather then complex offset calculations.
> > >
> > > This improved implementation should be more efficient and easier
> > > to understand and maintain.
> > >
> > > Tests Run(Both Pass):
> > > vchiq_test -p 1
> > > vchiq_test -f 10
> > >
> > > Signed-off-by: Michael Zoran <mzoran@crowfest.net>
> > > ---
> > > ?.../interface/vchiq_arm/vchiq_2835_arm.c???????????| 223
> > > +++++++++++----------
> > > ?1 file changed, 113 insertions(+), 110 deletions(-)
> >
> > This doesn't apply to the tree anymore because of your previous patch
> > :(
> >
> > Can you refresh it and resend?
> >
> > thanks,
> >
> > greg k-h
>
> OK, I resubmitted it.
>
> Once this patch gets applied 64-bit should be in decent shape. I'm not
> seeing any warnings or errors anymore and functional tests from a 64-
> bit OS look good.
>
> The only remaining issue that I know of is that it needs a 32-bit
> compatibility layer for the ioctls when running a 32-bit OS(Raspbian)
> on top of a 64-bit kernel.
Ok, let's turn off BROKEN for the module, and enable CONFIG_TEST and see
if the 0-day bot barfs all over it or not :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2 2/2] mmc: sdhci-iproc: support standard byte register accesses
From: Ulf Hansson @ 2016-11-10 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478018277-10097-3-git-send-email-scott.branden@broadcom.com>
On 1 November 2016 at 17:37, Scott Branden <scott.branden@broadcom.com> wrote:
> Add bytewise register accesses support for newer versions of IPROC
> SDHCI controllers.
> Previous sdhci-iproc versions of SDIO controllers
> (such as Raspberry Pi and Cygnus) only allowed for 32-bit register
> accesses.
>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-iproc.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 7262466..d7046d6 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -143,6 +143,14 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, u8 val, int reg)
> }
>
> static const struct sdhci_ops sdhci_iproc_ops = {
> + .set_clock = sdhci_set_clock,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> +static const struct sdhci_ops sdhci_iproc_32only_ops = {
> .read_l = sdhci_iproc_readl,
> .read_w = sdhci_iproc_readw,
> .read_b = sdhci_iproc_readb,
> @@ -156,6 +164,28 @@ static const struct sdhci_ops sdhci_iproc_ops = {
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
>
> +static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = {
> + .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> + .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
> + .ops = &sdhci_iproc_32only_ops,
> +};
> +
> +static const struct sdhci_iproc_data iproc_cygnus_data = {
> + .pdata = &sdhci_iproc_cygnus_pltfm_data,
> + .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> + & SDHCI_MAX_BLOCK_MASK) |
> + SDHCI_CAN_VDD_330 |
> + SDHCI_CAN_VDD_180 |
> + SDHCI_CAN_DO_SUSPEND |
> + SDHCI_CAN_DO_HISPD |
> + SDHCI_CAN_DO_ADMA2 |
> + SDHCI_CAN_DO_SDMA,
> + .caps1 = SDHCI_DRIVER_TYPE_C |
> + SDHCI_DRIVER_TYPE_D |
> + SDHCI_SUPPORT_DDR50,
> + .mmc_caps = MMC_CAP_1_8V_DDR,
> +};
> +
> static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN,
> @@ -182,7 +212,7 @@ static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = {
> .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> SDHCI_QUIRK_MISSING_CAPS,
> - .ops = &sdhci_iproc_ops,
> + .ops = &sdhci_iproc_32only_ops,
> };
>
> static const struct sdhci_iproc_data bcm2835_data = {
> @@ -194,7 +224,8 @@ static const struct sdhci_iproc_data bcm2835_data = {
>
> static const struct of_device_id sdhci_iproc_of_match[] = {
> { .compatible = "brcm,bcm2835-sdhci", .data = &bcm2835_data },
> - { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_data },
> + { .compatible = "brcm,sdhci-iproc-cygnus", .data = &iproc_cygnus_data},
> + { .compatible = "brcm,sdhci-iproc", .data = &iproc_data },
> { }
> };
> MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match);
> --
> 2.5.0
>
^ permalink raw reply
* [PATCH v2 1/2] mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings document
From: Ulf Hansson @ 2016-11-10 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478018277-10097-2-git-send-email-scott.branden@broadcom.com>
On 1 November 2016 at 17:37, Scott Branden <scott.branden@broadcom.com> wrote:
> Adds brcm,sdhci-iproc compat string to DT bindings document for
> the iProc SDHCI driver.
>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> index be56d2b..954561d 100644
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> @@ -7,6 +7,15 @@ Required properties:
> - compatible : Should be one of the following
> "brcm,bcm2835-sdhci"
> "brcm,sdhci-iproc-cygnus"
> + "brcm,sdhci-iproc"
> +
> +Use brcm2835-sdhci for Rasperry PI.
> +
> +Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
> +restricted to 32bit host accesses to SDHCI registers.
> +
> +Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
> +8, 16, 32-bit host access to SDHCI register.
>
> - clocks : The clock feeding the SDHCI controller.
>
> --
> 2.5.0
>
^ 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