Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic
From: Lorenzo Pieralisi @ 2016-11-14 16:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114155222.GZ2078@8bytes.org>

On Mon, Nov 14, 2016 at 04:52:23PM +0100, Joerg Roedel wrote:
> On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> > If we've already made the decision to move away from bus ops, I don't
> > see that it makes sense to deliberately introduce new dependencies on
> > them. Besides, as it stands, this patch literally implements "tell the
> > iommu-core which hardware-iommus exist in the system and a seperate
> > iommu_ops ptr for each of them" straight off.
> 
> Not sure which code you are looking at, but as I see it we have only
> per-device iommu-ops now (with this patch). That is different from
> having core-visible hardware-iommu instances where devices could link
> to.

This patch enables the IOMMU-OF-node<->device look-up on non-OF (ie
ACPI) systems by "converting" the of_node to a generic fwnode_handle,
that's all it does (and move the related look-up code from
drivers/iommu/of_iommu.c to drivers/iommu/iommu.c so that it does
not depend on OF_IOMMU any longer).

> Also the rest of iommu-core code still makes use of the per-bus ops. The
> per-device ops are only used for the of_xlate fn-ptr.

I can put this patch on the backburner and retrieve the iommu_ops
through the dev->bus path in the IORT xlate function (iort_iommu_xlate()
introduced in the last patch), the change is trivial and should work
just fine but it deserves a v8 to give everyone a chance to test it.

We would end-up handling the device->iommu_ops look-up differently in DT
and ACPI for streamid translations though, I am not sure I see a reason
why.

Thanks,
Lorenzo

^ permalink raw reply

* [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC
From: Lee Jones @ 2016-11-14 16:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9ba617f4-fd3d-2b48-2607-9eec3cc4253a@kernel.org>

On Sat, 12 Nov 2016, Jonathan Cameron wrote:

> On 10/11/16 16:18, Fabrice Gasnier wrote:
> > Add core driver for STMicroelectronics STM32 ADC (Analog to Digital
> > Converter). STM32 ADC can be composed of up to 3 ADCs with shared
> > resources like clock prescaler, common interrupt line and analog
> > reference voltage.
> > This core driver basically manages shared resources.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Looks good to me (other than the build issue obviously ;)
> 
> The fun bit will be trying to keep the whole thing this clean as you
> add the more 'interesting' functionality.  *fingers crossed*
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>

There isn't anything MFD about this driver.

Please move it into IIO.

> > ---
> >  drivers/mfd/Kconfig                |  14 ++
> >  drivers/mfd/Makefile               |   1 +
> >  drivers/mfd/stm32-adc-core.c       | 301 +++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/stm32-adc-core.h |  52 +++++++
> >  4 files changed, 368 insertions(+)
> >  create mode 100644 drivers/mfd/stm32-adc-core.c
> >  create mode 100644 include/linux/mfd/stm32-adc-core.h
> > 
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index c6df644..2580cee 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1152,6 +1152,20 @@ config MFD_PALMAS
> >  	  If you say yes here you get support for the Palmas
> >  	  series of PMIC chips from Texas Instruments.
> >  
> > +config MFD_STM32_ADC
> > +	tristate "STMicroelectronics STM32 adc"
> > +	depends on ARCH_STM32 || COMPILE_TEST
> > +	depends on OF
> > +	select MFD_CORE
> > +	select REGULATOR
> > +	select REGULATOR_FIXED_VOLTAGE
> > +	help
> > +	  Select this option to enable the core driver for STMicroelectronics
> > +	  STM32 analog-to-digital converter (ADC).
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called stm32-adc-core.
> > +
> >  config TPS6105X
> >  	tristate "TI TPS61050/61052 Boost Converters"
> >  	depends on I2C
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 9834e66..4571506 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS_PCI)	+= intel-lpss-pci.o
> >  obj-$(CONFIG_MFD_INTEL_LPSS_ACPI)	+= intel-lpss-acpi.o
> >  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
> >  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
> > +obj-$(CONFIG_MFD_STM32_ADC) 	+= stm32-adc-core.o
> >  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> >  obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
> >  obj-$(CONFIG_MFD_RK808)		+= rk808.o
> > diff --git a/drivers/mfd/stm32-adc-core.c b/drivers/mfd/stm32-adc-core.c
> > new file mode 100644
> > index 0000000..bcf52fb
> > --- /dev/null
> > +++ b/drivers/mfd/stm32-adc-core.c
> > @@ -0,0 +1,301 @@
> > +/*
> > + * This file is part of STM32 ADC driver
> > + *
> > + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> > + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> > + *
> > + * Inspired from: fsl-imx25-tsadc
> > + *
> > + * License type: GPLv2
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> > + * or FITNESS FOR A PARTICULAR PURPOSE.
> > + * See the GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/irqdesc.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/mfd/stm32-adc-core.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +
> > +/* STM32F4 - common registers for all ADC instances: 1, 2 & 3 */
> > +#define STM32F4_ADC_CSR			(STM32_ADCX_COMN_OFFSET + 0x00)
> > +#define STM32F4_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x04)
> > +
> > +/* STM32F4_ADC_CSR - bit fields */
> > +#define STM32F4_EOC3			BIT(17)
> > +#define STM32F4_EOC2			BIT(9)
> > +#define STM32F4_EOC1			BIT(1)
> > +
> > +/* STM32F4_ADC_CCR - bit fields */
> > +#define STM32F4_ADC_ADCPRE_SHIFT	16
> > +#define STM32F4_ADC_ADCPRE_MASK		GENMASK(17, 16)
> > +
> > +/* STM32 F4 maximum analog clock rate (from datasheet) */
> > +#define STM32F4_ADC_MAX_CLK_RATE	36000000
> > +
> > +/**
> > + * struct stm32_adc_priv - stm32 ADC core private data
> > + * @irq:		irq for ADC block
> > + * @domain:		irq domain reference
> > + * @aclk:		clock reference for the analog circuitry
> > + * @vref:		regulator reference
> > + * @common:		common data for all ADC instances
> > + */
> > +struct stm32_adc_priv {
> > +	int				irq;
> > +	struct irq_domain		*domain;
> > +	struct clk			*aclk;
> > +	struct regulator		*vref;
> > +	struct stm32_adc_common		common;
> > +};
> > +
> > +static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> > +{
> > +	return container_of(com, struct stm32_adc_priv, common);
> > +}
> > +
> > +/* STM32F4 ADC internal common clock prescaler division ratios */
> > +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
> > +
> > +/**
> > + * stm32f4_adc_clk_sel() - Select stm32f4 ADC common clock prescaler
> > + * @priv: stm32 ADC core private data
> > + * Select clock prescaler used for analog conversions, before using ADC.
> > + */
> > +static int stm32f4_adc_clk_sel(struct platform_device *pdev,
> > +			       struct stm32_adc_priv *priv)
> > +{
> > +	unsigned long rate;
> > +	u32 val;
> > +	int i;
> > +
> > +	rate = clk_get_rate(priv->aclk);
> > +	for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
> > +		if ((rate / stm32f4_pclk_div[i]) <= STM32F4_ADC_MAX_CLK_RATE)
> > +			break;
> > +	}
> > +	if (i >= ARRAY_SIZE(stm32f4_pclk_div))
> > +		return -EINVAL;
> > +
> > +	val = readl_relaxed(priv->common.base + STM32F4_ADC_CCR);
> > +	val &= ~STM32F4_ADC_ADCPRE_MASK;
> > +	val |= i << STM32F4_ADC_ADCPRE_SHIFT;
> > +	writel_relaxed(val, priv->common.base + STM32F4_ADC_CCR);
> > +
> > +	dev_dbg(&pdev->dev, "Using analog clock source at %ld kHz\n",
> > +		rate / (stm32f4_pclk_div[i] * 1000));
> > +
> > +	return 0;
> > +}
> > +
> > +/* ADC common interrupt for all instances */
> > +static void stm32_adc_irq_handler(struct irq_desc *desc)
> > +{
> > +	struct stm32_adc_priv *priv = irq_desc_get_handler_data(desc);
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	u32 status;
> > +
> > +	chained_irq_enter(chip, desc);
> > +	status = readl_relaxed(priv->common.base + STM32F4_ADC_CSR);
> > +
> > +	if (status & STM32F4_EOC1)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 0));
> > +
> > +	if (status & STM32F4_EOC2)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 1));
> > +
> > +	if (status & STM32F4_EOC3)
> > +		generic_handle_irq(irq_find_mapping(priv->domain, 2));
> > +
> > +	chained_irq_exit(chip, desc);
> > +};
> > +
> > +static int stm32_adc_domain_map(struct irq_domain *d, unsigned int irq,
> > +				irq_hw_number_t hwirq)
> > +{
> > +	irq_set_chip_data(irq, d->host_data);
> > +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_level_irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static void stm32_adc_domain_unmap(struct irq_domain *d, unsigned int irq)
> > +{
> > +	irq_set_chip_and_handler(irq, NULL, NULL);
> > +	irq_set_chip_data(irq, NULL);
> > +}
> > +
> > +static const struct irq_domain_ops stm32_adc_domain_ops = {
> > +	.map = stm32_adc_domain_map,
> > +	.unmap  = stm32_adc_domain_unmap,
> > +	.xlate = irq_domain_xlate_onecell,
> > +};
> > +
> > +static int stm32_adc_irq_probe(struct platform_device *pdev,
> > +			       struct stm32_adc_priv *priv)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +
> > +	priv->irq = platform_get_irq(pdev, 0);
> > +	if (priv->irq < 0) {
> > +		dev_err(&pdev->dev, "failed to get irq\n");
> > +		return priv->irq;
> > +	}
> > +
> > +	priv->domain = irq_domain_add_simple(np, STM32_ADC_MAX_ADCS, 0,
> > +					     &stm32_adc_domain_ops,
> > +					     priv);
> > +	if (!priv->domain) {
> > +		dev_err(&pdev->dev, "Failed to add irq domain\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	irq_set_chained_handler(priv->irq, stm32_adc_irq_handler);
> > +	irq_set_handler_data(priv->irq, priv);
> > +
> > +	return 0;
> > +}
> > +
> > +static void stm32_adc_irq_remove(struct platform_device *pdev,
> > +				 struct stm32_adc_priv *priv)
> > +{
> > +	int hwirq;
> > +
> > +	for (hwirq = 0; hwirq < STM32_ADC_MAX_ADCS; hwirq++)
> > +		irq_dispose_mapping(irq_find_mapping(priv->domain, hwirq));
> > +	irq_domain_remove(priv->domain);
> > +	irq_set_chained_handler(priv->irq, NULL);
> > +}
> > +
> > +static int stm32_adc_probe(struct platform_device *pdev)
> > +{
> > +	struct stm32_adc_priv *priv;
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct resource *res;
> > +	int ret;
> > +
> > +	if (!pdev->dev.of_node)
> > +		return -ENODEV;
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	priv->common.base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(priv->common.base))
> > +		return PTR_ERR(priv->common.base);
> > +
> > +	priv->vref = devm_regulator_get(&pdev->dev, "vref");
> > +	if (IS_ERR(priv->vref)) {
> > +		ret = PTR_ERR(priv->vref);
> > +		dev_err(&pdev->dev, "vref get failed, %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_enable(priv->vref);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "vref enable failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_get_voltage(priv->vref);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
> > +		goto err_regulator_disable;
> > +	}
> > +	priv->common.vref_mv = ret / 1000;
> > +	dev_dbg(&pdev->dev, "vref+=%dmV\n", priv->common.vref_mv);
> > +
> > +	priv->aclk = devm_clk_get(&pdev->dev, "adc");
> > +	if (IS_ERR(priv->aclk)) {
> > +		ret = PTR_ERR(priv->aclk);
> > +		dev_err(&pdev->dev, "Can't get 'adc' clock\n");
> > +		goto err_regulator_disable;
> > +	}
> > +
> > +	ret = clk_prepare_enable(priv->aclk);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "adc clk enable failed\n");
> > +		goto err_regulator_disable;
> > +	}
> > +
> > +	ret = stm32f4_adc_clk_sel(pdev, priv);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "adc clk selection failed\n");
> > +		goto err_clk_disable;
> > +	}
> > +
> > +	ret = stm32_adc_irq_probe(pdev, priv);
> > +	if (ret < 0)
> > +		goto err_clk_disable;
> > +
> > +	platform_set_drvdata(pdev, &priv->common);
> > +
> > +	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to populate DT children\n");
> > +		goto err_irq_remove;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_irq_remove:
> > +	stm32_adc_irq_remove(pdev, priv);
> > +
> > +err_clk_disable:
> > +	clk_disable_unprepare(priv->aclk);
> > +
> > +err_regulator_disable:
> > +	regulator_disable(priv->vref);
> > +
> > +	return ret;
> > +}
> > +
> > +static int stm32_adc_remove(struct platform_device *pdev)
> > +{
> > +	struct stm32_adc_common *common = platform_get_drvdata(pdev);
> > +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> > +
> > +	of_platform_depopulate(&pdev->dev);
> > +	stm32_adc_irq_remove(pdev, priv);
> > +	clk_disable_unprepare(priv->aclk);
> > +	regulator_disable(priv->vref);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id stm32_adc_of_match[] = {
> > +	{ .compatible = "st,stm32f4-adc-core" },
> > +};
> > +MODULE_DEVICE_TABLE(of, stm32_adc_of_match);
> > +
> > +static struct platform_driver stm32_adc_driver = {
> > +	.probe = stm32_adc_probe,
> > +	.remove = stm32_adc_remove,
> > +	.driver = {
> > +		.name = "stm32-adc-core",
> > +		.of_match_table = stm32_adc_of_match,
> > +	},
> > +};
> > +module_platform_driver(stm32_adc_driver);
> > +
> > +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> > +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC MFD driver");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:stm32-adc-core");
> > diff --git a/include/linux/mfd/stm32-adc-core.h b/include/linux/mfd/stm32-adc-core.h
> > new file mode 100644
> > index 0000000..081fa5f
> > --- /dev/null
> > +++ b/include/linux/mfd/stm32-adc-core.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * This file is part of STM32 ADC driver
> > + *
> > + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
> > + * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
> > + *
> > + * License type: GPLv2
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> > + * or FITNESS FOR A PARTICULAR PURPOSE.
> > + * See the GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef __STM32_ADC_H
> > +#define __STM32_ADC_H
> > +
> > +/*
> > + * STM32 - ADC global register map
> > + * ________________________________________________________
> > + * | Offset |                 Register                    |
> > + * --------------------------------------------------------
> > + * | 0x000  |                Master ADC1                  |
> > + * --------------------------------------------------------
> > + * | 0x100  |                Slave ADC2                   |
> > + * --------------------------------------------------------
> > + * | 0x200  |                Slave ADC3                   |
> > + * --------------------------------------------------------
> > + * | 0x300  |         Master & Slave common regs          |
> > + * --------------------------------------------------------
> > + */
> > +#define STM32_ADC_MAX_ADCS		3
> > +#define STM32_ADCX_COMN_OFFSET		0x300
> > +
> > +/**
> > + * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
> > + * @base:		control registers base cpu addr
> > + * @vref_mv:		vref voltage (mv)
> > + */
> > +struct stm32_adc_common {
> > +	void __iomem			*base;
> > +	int				vref_mv;
> > +};
> > +
> > +#endif
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Auger Eric @ 2016-11-14 16:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114162037.GC14829@8bytes.org>

Hi Joerg,

On 14/11/2016 17:20, Joerg Roedel wrote:
> On Mon, Nov 14, 2016 at 05:08:16PM +0100, Auger Eric wrote:
>> There are potentially several MSI doorbell physical pages in the SOC
>> that are accessed through the IOMMU (translated). Each of those must
>> have a corresponding IOVA and IOVA/PA mapping programmed in the IOMMU.
>> Else MSI will fault.
>>
>> - step 1 was to define a usable IOVA range for MSI mapping. So now we
>> decided the base address and size would be hardcoded for ARM. The
>> get_dm_region can be used to retrieve that hardcoded region.
>> - Step2 is to allocate IOVAs within that range and map then for each of
>> those MSI doorbells. This is done in the MSI controller compose() callback.
>>
>> I hope I succeeded in clarifying this time.
>>
>> Robin sent today a new version of its cookie think using a dummy
>> allocator. I am currently integrating it.
> 
> Okay, I understand. A simple bitmap-allocator would probably be
> sufficient, and doesn't have the overhead the iova allocator has. About
> how many pages are we talking here?
Very few actually. In the systems I have access to I only have a single
page. In most advanced systems we could imagine per-cpu doorbells but
this does not exist yet as far as I know.

Thanks

Eric
> 
> 
> 	Joerg
> 

^ permalink raw reply

* [PATCH RFC 3/2] ARM: improve arch_irq_work_has_interrupt()
From: Russell King - ARM Linux @ 2016-11-14 16:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2e14b943-e3a5-6993-48c7-68200c8b08a2@arm.com>

On Mon, Nov 14, 2016 at 04:30:57PM +0000, Marc Zyngier wrote:
> Hi Russell,
> 
> On 14/11/16 15:36, Russell King - ARM Linux wrote:
> > Following on from the previous patch, I think this makes more sense to
> > determine whether we can support IRQ work interrupts.
> > 
> > Whether we can support them or not depends on two things:
> > 
> > (a) whether the kernel has support for receiving IPIs
> > (b) whether it's possible to send an IPI to CPUs including the raising CPU.
> > 
> > (a) is a function of how the kernel is built - and in the case of ARM, it
> > depends whether the kernel is built with SMP enabled or not.
> > (b) is a property of the interrupt controller.
> > 
> > It hasn't ever been a function of the CPU or architecture.
> > 
> > Commit 059e232089e4 ("irqchip/gic: Allow self-SGIs for SMP on UP
> > configurations") changes the GIC IPI code such that we can raise
> > SGIs on uniprocessor systems running on a SMP kernel, which means
> > we can support IRQ work interrupts here as well.
> > 
> > So, we shouldn't be using cpu_smp() (or its previous is_smp() here
> > at all.  Use a flag to indicate whether we can IPI and use that to
> > indicate whether we support irq work interrupts.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > --- 
> >  arch/arm/include/asm/irq_work.h | 11 +++++++++--
> >  arch/arm/kernel/irq.c           |  0
> >  arch/arm/kernel/smp.c           |  3 +++
> >  drivers/irqchip/irq-gic.c       | 17 +++++++++++++----
> >  4 files changed, 25 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
> > index 2dc8d7995b48..d7262a3c2f2e 100644
> > --- a/arch/arm/include/asm/irq_work.h
> > +++ b/arch/arm/include/asm/irq_work.h
> > @@ -1,11 +1,18 @@
> >  #ifndef __ASM_ARM_IRQ_WORK_H
> >  #define __ASM_ARM_IRQ_WORK_H
> >  
> > -#include <asm/smp_plat.h>
> > +extern bool irq_controller_can_ipi;
> > +#define irq_controller_can_ipi irq_controller_can_ipi
> >  
> >  static inline bool arch_irq_work_has_interrupt(void)
> >  {
> > -	return cpu_smp();
> > +#ifdef CONFIG_SMP
> > +	/* This depends on the IRQ controller */
> > +	return irq_controller_can_ipi;
> > +#else
> > +	/* The kernel is not built to support IPIs */
> > +	return false;
> > +#endif
> >  }
> >  
> >  #endif /* _ASM_ARM_IRQ_WORK_H */
> > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> > index 7dd14e8395e6..1fa9412cc4aa 100644
> > --- a/arch/arm/kernel/smp.c
> > +++ b/arch/arm/kernel/smp.c
> > @@ -473,6 +473,9 @@ void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
> >  		__smp_cross_call = fn;
> >  }
> >  
> > +/* This indicates whether the IRQ controller can IPI (including self-IPI) */
> > +bool irq_controller_can_ipi;
> 
> We probably need to initialize this to false, since we have at least 4
> other users of set_smp_cross_call() in the tree.

C programming 101: BSS variables are initialised to zero at the start
of the program.

> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> > index d6c404b3584d..abe8d5807c0f 100644
> > --- a/drivers/irqchip/irq-gic.c
> > +++ b/drivers/irqchip/irq-gic.c
> > @@ -1187,9 +1187,6 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
> >  		 */
> >  		for (i = 0; i < NR_GIC_CPU_IF; i++)
> >  			gic_cpu_map[i] = 0xff;
> > -#ifdef CONFIG_SMP
> > -		set_smp_cross_call(gic_raise_softirq);
> > -#endif
> >  		cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_GIC_STARTING,
> >  					  "AP_IRQ_GIC_STARTING",
> >  					  gic_starting_cpu, NULL);
> > @@ -1207,8 +1204,20 @@ static int __init __gic_init_bases(struct gic_chip_data *gic,
> >  	}
> >  
> >  	ret = gic_init_bases(gic, irq_start, handle);
> > -	if (ret)
> > +	if (ret) {
> >  		kfree(name);
> > +		return ret;
> > +	}
> > +
> > +	if (gic == &gic_data[0]) {
> > +#ifdef CONFIG_SMP
> > +		set_smp_cross_call(gic_raise_softirq);
> > +#ifdef irq_controller_can_ipi
> > +		if (nr_cpu_ids == 1 || hweight8(gic_cpu_map[0]) == 1)
> > +			irq_controller_can_ipi = true;
> 
> Am I missing something, or is there any sane configuration where this 
> isn't true?

I hope not, but I want to duplicate here the conditions where
gic_raise_softirq() actually _works_ so we stop running into corner
cases where we get "irq work fails" etc.

> Also, maybe it would make some sense to have a more 
> streamlined interface to the architecture code. Something along the 
> lines of:
> 
> diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
> index 3d6dc8b..45612d2 100644
> --- a/arch/arm/include/asm/smp.h
> +++ b/arch/arm/include/asm/smp.h
> @@ -48,6 +48,16 @@ extern void smp_init_cpus(void);
>   */
>  extern void set_smp_cross_call(void (*)(const struct cpumask *, unsigned int));
>  
> +#ifdef CONFIG_SMP
> +#define setup_smp_ipi(f,i)			\
> +	do {					\
> +		set_smp_cross_call(f);		\
> +		irq_controller_can_ipi = (i);	\
> +	} while(0)
> +#else
> +#define setup_smp_ipi(f,i)	do { } while (0)
> +#endif
> +
>  /*
>   * Called from platform specific assembly code, this is the
>   * secondary CPU entry point.
> 
> with the similar entry point for arm64?

I'd prefer to keep the two things separate, but we should definitely
provide a stub for set_smp_cross_call() for when SMP is disabled.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v3 1/3] Documentation: DT: add dma compatible for sun50i A64 SOC.
From: Rob Herring @ 2016-11-14 17:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161107181457.GA3619@arx12>

On Tue, Nov 08, 2016 at 02:14:57AM +0800, Hao Zhang wrote:
> This adds documentation of the sun50i a64 dma binding compatible.
> 
> Signed-off-by: Hao Zhang <hao5781286@gmail.com>
> ---
>  Documentation/devicetree/bindings/dma/sun6i-dma.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 01/30] usb: dwc2: Deprecate g-use-dma binding
From: Rob Herring @ 2016-11-14 17:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6e90b835-73b1-3970-24a4-eab72381b469@synopsys.com>

On Tue, Nov 08, 2016 at 09:48:03AM -0800, John Youn wrote:
> On 11/8/2016 1:12 AM, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > John Youn <johnyoun@synopsys.com> writes:
> >> Add a vendor prefix and make the name more consistent by renaming it to
> >> "snps,gadget-dma-enable".
> >>
> >> Signed-off-by: John Youn <johnyoun@synopsys.com>
> >> ---
> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 5 ++++-
> >>  arch/arm/boot/dts/rk3036.dtsi                  | 2 +-
> >>  arch/arm/boot/dts/rk3288.dtsi                  | 2 +-
> >>  arch/arm/boot/dts/rk3xxx.dtsi                  | 2 +-
> >>  arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 2 +-
> >>  arch/arm64/boot/dts/rockchip/rk3368.dtsi       | 2 +-
> >>  drivers/usb/dwc2/params.c                      | 9 ++++++++-
> >>  drivers/usb/dwc2/pci.c                         | 2 +-
> >>  8 files changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> index 9472111..389a461 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >> @@ -26,11 +26,14 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
> >>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>    Refer to usb/generic.txt
> >>  - snps,host-dma-disable: disable host DMA mode.
> >> -- g-use-dma: enable dma usage in gadget driver.
> >> +- snps,gadget-dma-enable: enable gadget DMA mode.
> > 
> > I don't see why you even have this binding. Looking through the code,
> > you have:
> > 
> > #define GHWCFG2_SLAVE_ONLY_ARCH			0
> > #define GHWCFG2_EXT_DMA_ARCH			1
> > #define GHWCFG2_INT_DMA_ARCH			2
> > 
> > void dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val)
> > {
> > 	int valid = 1;
> > 
> > 	if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH)
> > 		valid = 0;
> > 	if (val < 0)
> > 		valid = 0;
> > 
> > 	if (!valid) {
> > 		if (val >= 0)
> > 			dev_err(hsotg->dev,
> > 				"%d invalid for dma_enable parameter. Check HW configuration.\n",
> > 				val);
> > 		val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH;
> > 		dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val);
> > 	}
> > 
> > 	hsotg->core_params->dma_enable = val;
> > }
> > 
> > which seems to hint that DMA support is discoverable. If there is DMA,
> > why would disable it?
> > 
> 
> Yes that's the case and I would prefer to make it discoverable and
> enabled by default.
> 
> But the legacy behavior has always been like this because DMA was
> never fully implemented in the gadget driver and it was an opt-in
> feature. Periodic support was only added recently.
> 
> What do you think about enabling it by default now? I think most
> platforms already use DMA.
> 
> We would still need a "disable" binding for IP validation purposes at
> least.

You can hack up your kernel for that. You may need a disable for broken 
h/w perhaps.

Rob

^ permalink raw reply

* [PATCH v6 4/9] drm/hisilicon/hibmc: Add plane for DE
From: Sean Paul @ 2016-11-14 17:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5826A498.4000002@huawei.com>

On Sat, Nov 12, 2016 at 12:11 AM, Rongrong Zou <zourongrong@huawei.com> wrote:
> ? 2016/11/11 5:53, Sean Paul ??:
>>
>> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com>
>> wrote:
>>>
>>> Add plane funcs and helper funcs for DE.
>>>
>>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>>> ---
>>>   drivers/gpu/drm/hisilicon/hibmc/Kconfig         |   1 +
>>>   drivers/gpu/drm/hisilicon/hibmc/Makefile        |   2 +-
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  | 170
>>> ++++++++++++++++++++++++
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.h  |  29 ++++
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c |  51 ++++++-
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |   5 +
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c     |   6 +
>>>   7 files changed, 261 insertions(+), 3 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.h
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> index bcb8c18..380622a 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
>>> @@ -1,6 +1,7 @@
>>>   config DRM_HISI_HIBMC
>>>          tristate "DRM Support for Hisilicon Hibmc"
>>>          depends on DRM && PCI
>>> +       select DRM_KMS_HELPER
>>>          select DRM_TTM
>>>
>>>          help
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> index 810a37e..72e107e 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>>> @@ -1,5 +1,5 @@
>>>   ccflags-y := -Iinclude/drm
>>> -hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_fbdev.o hibmc_drm_power.o
>>> hibmc_ttm.o
>>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_fbdev.o
>>> hibmc_drm_power.o hibmc_ttm.o
>>>
>>>   obj-$(CONFIG_DRM_HISI_HIBMC)   +=hibmc-drm.o
>>>   #obj-y += hibmc-drm.o
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> new file mode 100644
>>> index 0000000..9c1a68c
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> @@ -0,0 +1,170 @@
>>> +/* 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 <drm/drm_atomic.h>
>>> +#include <drm/drm_atomic_helper.h>
>>> +#include <drm/drm_crtc_helper.h>
>>> +#include <drm/drm_plane_helper.h>
>>> +
>>> +#include "hibmc_drm_drv.h"
>>> +#include "hibmc_drm_regs.h"
>>> +#include "hibmc_drm_power.h"
>>> +
>>> +/*
>>> ---------------------------------------------------------------------- */
>>
>>
>> Remove
>
>
> ok, will do, thanks.
>
>>
>>> +
>>> +static int hibmc_plane_atomic_check(struct drm_plane *plane,
>>> +                                   struct drm_plane_state *state)
>>> +{
>>> +       struct drm_framebuffer *fb = state->fb;
>>> +       struct drm_crtc *crtc = state->crtc;
>>> +       struct drm_crtc_state *crtc_state;
>>> +       u32 src_x = state->src_x >> 16;
>>> +       u32 src_y = state->src_y >> 16;
>>> +       u32 src_w = state->src_w >> 16;
>>> +       u32 src_h = state->src_h >> 16;
>>> +       int crtc_x = state->crtc_x;
>>> +       int crtc_y = state->crtc_y;
>>> +       u32 crtc_w = state->crtc_w;
>>> +       u32 crtc_h = state->crtc_h;
>>
>>
>> I don't think you gain anything with the crtc_* vars
>
>
> It would work well, but looks redundant and not simple enough,
> will delete them, thanks.
>
>>
>>> +
>>> +       if (!crtc || !fb)
>>> +               return 0;
>>> +
>>> +       crtc_state = drm_atomic_get_crtc_state(state->state, crtc);
>>> +       if (IS_ERR(crtc_state))
>>> +               return PTR_ERR(crtc_state);
>>> +
>>> +       if (src_w != crtc_w || src_h != crtc_h) {
>>> +               DRM_ERROR("Scale not support!!!\n");
>>
>>
>> I like the enthusiasm, but I think DRM_DEBUG_ATOMIC would be better
>
>
> I'm sorry, can you explain why here should be an DRM_DEBUG_ATOMIC,
> when this condition is hit, it is really an error and atomic_commit will
> abort with failure.
>

I don't have strong opinions, but this class of failure isn't a driver
error, so much as invalid input from userspace. As such, I'd tend to
classify it as debug level.

At any rate, keep it ERROR if you really want.

Sean

>>
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       if (src_x + src_w > fb->width ||
>>> +           src_y + src_h > fb->height)
>>
>>
>> These should be already covered in drm_atomic_plane_check
>
>
> understood, thanks.
>
>>
>>> +               return -EINVAL;
>>> +
>>> +       if (crtc_x < 0 || crtc_y < 0)
>>
>>
>> Print DRM_DEBUG_ATOMIC message here
>
>
> agreed. thanks.
>
>>
>>> +               return -EINVAL;
>>> +
>>> +       if (crtc_x + crtc_w > crtc_state->adjusted_mode.hdisplay ||
>>> +           crtc_y + crtc_h > crtc_state->adjusted_mode.vdisplay)
>>
>>
>> DRM_DEBUG_ATOMIC here too
>
>
> ditto.
>
>>
>>> +               return -EINVAL;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void hibmc_plane_atomic_update(struct drm_plane *plane,
>>> +                                     struct drm_plane_state *old_state)
>>> +{
>>> +       struct drm_plane_state  *state  = plane->state;
>>> +       u32 reg;
>>> +       int ret;
>>> +       u64 gpu_addr = 0;
>>> +       unsigned int line_l;
>>> +       struct hibmc_drm_device *hidev =
>>> +               (struct hibmc_drm_device *)plane->dev->dev_private;
>>> +
>>
>>
>> nit: extra line
>
>
> will delete, thanks.
>
>>> +       struct hibmc_framebuffer *hibmc_fb;
>>> +       struct hibmc_bo *bo;
>>> +
>>> +       hibmc_fb = to_hibmc_framebuffer(state->fb);
>>> +       bo = gem_to_hibmc_bo(hibmc_fb->obj);
>>> +       ret = ttm_bo_reserve(&bo->bo, true, false, NULL);
>>> +       if (ret)
>>
>>
>> Print error
>
>
> agreed, thanks.
>
>>
>>> +               return;
>>> +
>>> +       hibmc_bo_pin(bo, TTM_PL_FLAG_VRAM, &gpu_addr);
>>
>>
>> Check return value
>
>
> ok, thanks.
>
>>
>>> +       if (ret) {
>>> +               ttm_bo_unreserve(&bo->bo);
>>> +               return;
>>> +       }
>>> +
>>> +       ttm_bo_unreserve(&bo->bo);
>>
>>
>> Move this up before the conditional so you don't have to call it in
>> both branches
>
>
> understood, thanks.
>
>>
>>> +
>>> +       writel(gpu_addr, hidev->mmio + HIBMC_CRT_FB_ADDRESS);
>>> +
>>> +       reg = state->fb->width * (state->fb->bits_per_pixel >> 3);
>>> +       /* now line_pad is 16 */
>>> +       reg = PADDING(16, reg);
>>> +
>>> +       line_l = state->fb->width * state->fb->bits_per_pixel / 8;
>>
>>
>> above, you >> 3. here you / 8, pick one?
>
>
> i prefer /8 because it is more readable to human, although it is less
> effective
> in executing.
>

I think the compiler will optimize it, regardless.

Sean

>>
>>> +       line_l = PADDING(16, line_l);
>>> +       writel((HIBMC_CRT_FB_WIDTH_WIDTH(reg) &
>>> HIBMC_CRT_FB_WIDTH_WIDTH_MASK) |
>>> +              (HIBMC_CRT_FB_WIDTH_OFFS(line_l) &
>>> HIBMC_CRT_FB_WIDTH_OFFS_MASK),
>>> +              hidev->mmio + HIBMC_CRT_FB_WIDTH);
>>> +
>>> +       /* SET PIXEL FORMAT */
>>> +       reg = readl(hidev->mmio + HIBMC_CRT_DISP_CTL);
>>> +       reg = reg & ~HIBMC_CRT_DISP_CTL_FORMAT_MASK;
>>> +       reg = reg | (HIBMC_CRT_DISP_CTL_FORMAT(state->fb->bits_per_pixel
>>> >> 4) &
>>> +                    HIBMC_CRT_DISP_CTL_FORMAT_MASK);
>>> +       writel(reg, hidev->mmio + HIBMC_CRT_DISP_CTL);
>>> +}
>>> +
>>> +static void hibmc_plane_atomic_disable(struct drm_plane *plane,
>>> +                                      struct drm_plane_state *old_state)
>>> +{
>>> +}
>>
>>
>> The caller checks for NULL, no need to stub
>
>
> thanks for pointing it out,
> will remove.
>
> Regards,
> Rongrong.
>
>>
>>> +
>>> +static const u32 channel_formats1[] = {
>>> +       DRM_FORMAT_RGB565, DRM_FORMAT_BGR565, DRM_FORMAT_RGB888,
>>> +       DRM_FORMAT_BGR888, DRM_FORMAT_XRGB8888, DRM_FORMAT_XBGR8888,
>>> +       DRM_FORMAT_RGBA8888, DRM_FORMAT_BGRA8888, DRM_FORMAT_ARGB8888,
>>> +       DRM_FORMAT_ABGR8888
>>> +};
>>> +
>>> +static struct drm_plane_funcs hibmc_plane_funcs = {
>>> +       .update_plane   = drm_atomic_helper_update_plane,
>>> +       .disable_plane  = drm_atomic_helper_disable_plane,
>>> +       .set_property = drm_atomic_helper_plane_set_property,
>>> +       .destroy = drm_plane_cleanup,
>>> +       .reset = drm_atomic_helper_plane_reset,
>>> +       .atomic_duplicate_state =
>>> drm_atomic_helper_plane_duplicate_state,
>>> +       .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>>> +};
>>> +
>>> +static const struct drm_plane_helper_funcs hibmc_plane_helper_funcs = {
>>> +       .atomic_check = hibmc_plane_atomic_check,
>>> +       .atomic_update = hibmc_plane_atomic_update,
>>> +       .atomic_disable = hibmc_plane_atomic_disable,
>>> +};
>>> +
>>> +int hibmc_plane_init(struct hibmc_drm_device *hidev)
>>> +{
>>> +       struct drm_device *dev = hidev->dev;
>>> +       struct drm_plane *plane = &hidev->plane;
>>> +       int ret = 0;
>>> +
>>> +       /*
>>> +        * plane init
>>> +        * TODO: Now only support primary plane, overlay planes
>>> +        * need to do.
>>> +        */
>>> +       ret = drm_universal_plane_init(dev, plane, 1, &hibmc_plane_funcs,
>>> +                                      channel_formats1,
>>> +                                      ARRAY_SIZE(channel_formats1),
>>> +                                      DRM_PLANE_TYPE_PRIMARY,
>>> +                                      NULL);
>>> +       if (ret) {
>>> +               DRM_ERROR("fail to init plane!!!\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       drm_plane_helper_add(plane, &hibmc_plane_helper_funcs);
>>> +       return 0;
>>> +}
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.h
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.h
>>> new file mode 100644
>>> index 0000000..4ce0d7b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.h
>>> @@ -0,0 +1,29 @@
>>> +/* 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_DE_H
>>> +#define HIBMC_DRM_DE_H
>>> +
>>> +struct panel_pll {
>>> +       unsigned long M;
>>> +       unsigned long N;
>>> +       unsigned long OD;
>>> +       unsigned long POD;
>>> +};
>>> +
>>> +#endif
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> index 5ac7a7e..7d96583 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -18,6 +18,7 @@
>>>
>>>   #include <linux/module.h>
>>>   #include <linux/console.h>
>>> +#include <drm/drm_crtc_helper.h>
>>>
>>>   #include "hibmc_drm_drv.h"
>>>   #include "hibmc_drm_regs.h"
>>> @@ -47,8 +48,8 @@ static void hibmc_disable_vblank(struct drm_device
>>> *dev, unsigned int pipe)
>>>   }
>>>
>>>   static struct drm_driver hibmc_driver = {
>>> -       .driver_features        = DRIVER_GEM,
>>> -
>>> +       .driver_features        = DRIVER_GEM | DRIVER_MODESET |
>>> +                                 DRIVER_ATOMIC,
>>>          .fops                   = &hibmc_fops,
>>>          .name                   = "hibmc",
>>>          .date                   = "20160828",
>>> @@ -70,6 +71,7 @@ static int hibmc_pm_suspend(struct device *dev)
>>>          struct drm_device *drm_dev = pci_get_drvdata(pdev);
>>>          struct hibmc_drm_device *hidev = drm_dev->dev_private;
>>>
>>> +       drm_kms_helper_poll_disable(drm_dev);
>>>          drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 1);
>>>
>>>          return 0;
>>> @@ -81,7 +83,9 @@ static int hibmc_pm_resume(struct device *dev)
>>>          struct drm_device *drm_dev = pci_get_drvdata(pdev);
>>>          struct hibmc_drm_device *hidev = drm_dev->dev_private;
>>>
>>> +       drm_helper_resume_force_mode(drm_dev);
>>>          drm_fb_helper_set_suspend_unlocked(&hidev->fbdev->helper, 0);
>>> +       drm_kms_helper_poll_enable(drm_dev);
>>>
>>>          return 0;
>>>   }
>>> @@ -91,6 +95,41 @@ static int hibmc_pm_resume(struct device *dev)
>>>                                  hibmc_pm_resume)
>>>   };
>>>
>>> +static int hibmc_kms_init(struct hibmc_drm_device *hidev)
>>> +{
>>> +       int ret;
>>> +
>>> +       drm_mode_config_init(hidev->dev);
>>> +       hidev->mode_config_initialized = true;
>>> +
>>> +       hidev->dev->mode_config.min_width = 0;
>>> +       hidev->dev->mode_config.min_height = 0;
>>> +       hidev->dev->mode_config.max_width = 1920;
>>> +       hidev->dev->mode_config.max_height = 1440;
>>> +
>>> +       hidev->dev->mode_config.fb_base = hidev->fb_base;
>>> +       hidev->dev->mode_config.preferred_depth = 24;
>>> +       hidev->dev->mode_config.prefer_shadow = 0;
>>> +
>>> +       hidev->dev->mode_config.funcs = (void *)&hibmc_mode_funcs;
>>> +
>>> +       ret = hibmc_plane_init(hidev);
>>> +       if (ret) {
>>> +               DRM_ERROR("fail to init plane!!!\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void hibmc_kms_fini(struct hibmc_drm_device *hidev)
>>> +{
>>> +       if (hidev->mode_config_initialized) {
>>> +               drm_mode_config_cleanup(hidev->dev);
>>> +               hidev->mode_config_initialized = false;
>>> +       }
>>> +}
>>> +
>>>   static int hibmc_hw_config(struct hibmc_drm_device *hidev)
>>>   {
>>>          unsigned int reg;
>>> @@ -183,6 +222,7 @@ static int hibmc_unload(struct drm_device *dev)
>>>          struct hibmc_drm_device *hidev = dev->dev_private;
>>>
>>>          hibmc_fbdev_fini(hidev);
>>> +       hibmc_kms_fini(hidev);
>>>          hibmc_mm_fini(hidev);
>>>          hibmc_hw_fini(hidev);
>>>          dev->dev_private = NULL;
>>> @@ -208,6 +248,13 @@ static int hibmc_load(struct drm_device *dev,
>>> unsigned long flags)
>>>          if (ret)
>>>                  goto err;
>>>
>>> +       ret = hibmc_kms_init(hidev);
>>> +       if (ret)
>>> +               goto err;
>>> +
>>> +       /* reset all the states of crtc/plane/encoder/connector */
>>> +       drm_mode_config_reset(dev);
>>> +
>>>          ret = hibmc_fbdev_init(hidev);
>>>          if (ret)
>>>                  goto err;
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> index a40e9a7..49e39d2 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> @@ -45,6 +45,8 @@ struct hibmc_drm_device {
>>>
>>>          /* drm */
>>>          struct drm_device  *dev;
>>> +       struct drm_plane plane;
>>> +       bool mode_config_initialized;
>>>
>>>          /* ttm */
>>>          struct {
>>> @@ -82,6 +84,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct
>>> drm_gem_object *gem)
>>>
>>>   #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>>
>>> +int hibmc_plane_init(struct hibmc_drm_device *hidev);
>>>   int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>>>   void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>>>
>>> @@ -102,4 +105,6 @@ int hibmc_dumb_mmap_offset(struct drm_file *file,
>>> struct drm_device *dev,
>>>                             u32 handle, u64 *offset);
>>>   int hibmc_mmap(struct file *filp, struct vm_area_struct *vma);
>>>
>>> +extern const struct drm_mode_config_funcs hibmc_mode_funcs;
>>> +
>>>   #endif
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> index 9822f62..beb4d76 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
>>> @@ -554,3 +554,9 @@ struct hibmc_framebuffer *
>>>          }
>>>          return &hibmc_fb->fb;
>>>   }
>>> +
>>> +const struct drm_mode_config_funcs hibmc_mode_funcs = {
>>> +       .atomic_check = drm_atomic_helper_check,
>>> +       .atomic_commit = drm_atomic_helper_commit,
>>> +       .fb_create = hibmc_user_framebuffer_create,
>>> +};
>>> --
>>> 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
>>
>> .
>>
>
> _______________________________________________
> 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 2/2] ARM: dts: vf610-zii-dev: Add .dts file for rev. C
From: Andrew Lunn @ 2016-11-14 17:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479141306-15141-2-git-send-email-andrew.smirnov@gmail.com>

> +		mdio_mux_1: mdio at 1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			switch0: switch0 at 0 {
> +				compatible = "marvell,mv88e6390";

Hi Andrey

The driver for this is not in net-next yet. And when it is, it will
probably be called "marvell,mv88e6190", keeping to the pattern of the
lowest product ID which supports these features.

> +					port at 4 {
> +						reg = <4>;
> +						label = "lan4";
> +					};
> +
> +					port at 9 {
> +						reg = <9>;
> +						label = "lan4";
> +						phy-handle = <&switch0phy0>;
> +					};
> +

You have two "lan4". I would also suggest leaving port 9 out for the
moment. It needs clause 45 MDIO to talk to the PHY, which we don't
have yet. Hence it cannot find it, and so give an error.

> +
> +					switch0port10: port at 10 {
> +						reg = <10>;
> +						label = "dsa";
> +						phy-mode = "xgmii";
> +						link = <&switch1port10>;
> +						fixed-link {
> +							speed = <10000>;
> +							full-duplex;
> +						};

This fixed-link node is wrong, and invalid. 10000 is not supported by
the fixed link driver, only 10, 100, and 1000. Also, it is not
required. The DSA driver should configure the link to the fastest
possible speed the port supports. You only need a fixed-link property
when you need to configure it at a lower speed. Rev B also gets this
wrong.

> +					};
> +				};
> +
> +				mdio {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					switch0phy0: switch0phy0 at 0 {
> +						reg = <0>;
> +					};

I think the strapping for the PHY is such that it is at address 9.
Also, it is on the external mdio bus, not the internal mdio bus. The
6390 family has two MDIO busses. I have patches to support this, which
will appear eventually.

  Andrew

^ permalink raw reply

* [PATCH v6 5/9] drm/hisilicon/hibmc: Add crtc for DE
From: Sean Paul @ 2016-11-14 17:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5826ECA6.6030900@huawei.com>

On Sat, Nov 12, 2016 at 5:19 AM, Rongrong Zou <zourongrong@huawei.com> wrote:
> ? 2016/11/11 6:14, Sean Paul ??:
>>
>> On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@gmail.com>
>> wrote:
>>>
>>> Add crtc funcs and helper funcs for DE.
>>>
>>> Signed-off-by: Rongrong Zou <zourongrong@gmail.com>
>>> ---
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  | 318
>>> ++++++++++++++++++++++++
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c |   6 +
>>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h |   2 +
>>>   3 files changed, 326 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> index 9c1a68c..9b5d0d0 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
>>> @@ -23,6 +23,7 @@
>>>
>>>   #include "hibmc_drm_drv.h"
>>>   #include "hibmc_drm_regs.h"
>>> +#include "hibmc_drm_de.h"
>>>   #include "hibmc_drm_power.h"
>>
>>
>> nit: alphabetize
>
>
> ok, thanks.
>
>>
>>>
>>>   /*
>>> ---------------------------------------------------------------------- */
>>
>>
>> Remove
>
>
> will do, thanks.
>
>>
>>> @@ -168,3 +169,320 @@ int hibmc_plane_init(struct hibmc_drm_device
>>> *hidev)
>>>          drm_plane_helper_add(plane, &hibmc_plane_helper_funcs);
>>>          return 0;
>>>   }
>>> +
>>> +static void hibmc_crtc_enable(struct drm_crtc *crtc)
>>> +{
>>> +       unsigned int reg;
>>> +       /* power mode 0 is default. */
>>
>>
>> This comment seems to be in the wrong place
>
>
> will remove it, thanks.
>
>
>>
>>> +       struct hibmc_drm_device *hidev = crtc->dev->dev_private;
>>> +
>>> +       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_LOCALMEM_MASK;
>>> +       reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
>>> +       reg |= HIBMC_CURR_GATE_LOCALMEM(ON);
>>> +       reg |= HIBMC_CURR_GATE_DISPLAY(ON);
>>> +       hibmc_set_current_gate(hidev, reg);
>>> +       drm_crtc_vblank_on(crtc);
>>> +}
>>> +
>>> +static void hibmc_crtc_disable(struct drm_crtc *crtc)
>>> +{
>>> +       unsigned int reg;
>>> +       struct hibmc_drm_device *hidev = crtc->dev->dev_private;
>>> +
>>> +       drm_crtc_vblank_off(crtc);
>>> +
>>> +       hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_SLEEP);
>>> +
>>> +       /* Enable display power gate & LOCALMEM power gate*/
>>> +       reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
>>> +       reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
>>> +       reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
>>> +       reg |= HIBMC_CURR_GATE_LOCALMEM(OFF);
>>> +       reg |= HIBMC_CURR_GATE_DISPLAY(OFF);
>>> +       hibmc_set_current_gate(hidev, reg);
>>> +}
>>> +
>>> +static int hibmc_crtc_atomic_check(struct drm_crtc *crtc,
>>> +                                  struct drm_crtc_state *state)
>>> +{
>>> +       return 0;
>>> +}
>>
>>
>> Caller NULL-checks, no need for stub
>
>
> thanks for pointing it out.
>
>
>>
>>> +
>>> +static unsigned int format_pll_reg(void)
>>> +{
>>> +       unsigned int pllreg = 0;
>>> +       struct panel_pll pll = {0};
>>> +
>>> +       /* Note that all PLL's have the same format. Here,
>>> +        * we just use Panel PLL parameter to work out the bit
>>> +        * fields in the register.On returning a 32 bit number, the value
>>> can
>>> +        * be applied to any PLL in the calling function.
>>> +        */
>>> +       pllreg |= HIBMC_PLL_CTRL_BYPASS(OFF) &
>>> HIBMC_PLL_CTRL_BYPASS_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_POWER(ON) & HIBMC_PLL_CTRL_POWER_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_INPUT(OSC) & HIBMC_PLL_CTRL_INPUT_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_POD(pll.POD) & HIBMC_PLL_CTRL_POD_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_OD(pll.OD) & HIBMC_PLL_CTRL_OD_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_N(pll.N) & HIBMC_PLL_CTRL_N_MASK;
>>> +       pllreg |= HIBMC_PLL_CTRL_M(pll.M) & HIBMC_PLL_CTRL_M_MASK;
>>> +
>>> +       return pllreg;
>>> +}
>>> +
>>> +static void set_vclock_hisilicon(struct drm_device *dev, unsigned long
>>> pll)
>>> +{
>>> +       unsigned long tmp0, tmp1;
>>> +       struct hibmc_drm_device *hidev = dev->dev_private;
>>> +
>>> +       /* 1. outer_bypass_n=0 */
>>> +       tmp0 = readl(hidev->mmio + CRT_PLL1_HS);
>>> +       tmp0 &= 0xBFFFFFFF;
>>> +       writel(tmp0, hidev->mmio + CRT_PLL1_HS);
>>> +
>>> +       /* 2. pll_pd=1?inter_bypass=1 */
>>> +       writel(0x21000000, hidev->mmio + CRT_PLL1_HS);
>>> +
>>> +       /* 3. config pll */
>>> +       writel(pll, hidev->mmio + CRT_PLL1_HS);
>>> +
>>> +       /* 4. delay  */
>>> +       mdelay(1);
>>
>>
>> These should be usleep_range() see
>> https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
>
>
> This looks better to me. i think a 'usleep_range(1000, 2000)' is ok.
>
>>
>>> +
>>> +       /* 5. pll_pd =0 */
>>> +       tmp1 = pll & ~0x01000000;
>>> +       writel(tmp1, hidev->mmio + CRT_PLL1_HS);
>>> +
>>> +       /* 6. delay  */
>>> +       mdelay(1);
>>> +
>>> +       /* 7. inter_bypass=0 */
>>> +       tmp1 &= ~0x20000000;
>>> +       writel(tmp1, hidev->mmio + CRT_PLL1_HS);
>>> +
>>> +       /* 8. delay  */
>>> +       mdelay(1);
>>> +
>>> +       /* 9. outer_bypass_n=1 */
>>> +       tmp1 |= 0x40000000;
>>> +       writel(tmp1, hidev->mmio + CRT_PLL1_HS);
>>
>>
>> This function is a whole lot of magic. Any chance you can pull the
>> values out into #defines?
>
>
> will do. thanks.
>
>>
>>> +}
>>> +
>>> +/* This function takes care the extra registers and bit fields required
>>> to
>>
>>
>> nit: multi-line comments have a leading /* line with the comment
>> starting on the following line
>
>
> thanks for pointing it out.
>
>>
>> applies below as well
>>
>>
>>> + *setup a mode in board.
>>
>>
>> nit: space between * and comment, ie: * setup a mode in board
>
>
> understood, thanks.
>
>
>>
>> applies to the rest of the comment too
>>
>>
>>> + *Explanation about Display Control register:
>>> + *FPGA only supports 7 predefined pixel clocks, and clock select is
>>> + *in bit 4:0 of new register 0x802a8.
>>> + */
>>> +static unsigned int display_ctrl_adjust(struct drm_device *dev,
>>> +                                       struct drm_display_mode *mode,
>>> +                                       unsigned int ctrl)
>>> +{
>>> +       unsigned long x, y;
>>> +       unsigned long pll1; /* bit[31:0] of PLL */
>>> +       unsigned long pll2; /* bit[63:32] of PLL */
>>> +       struct hibmc_drm_device *hidev = dev->dev_private;
>>> +
>>> +       x = mode->hdisplay;
>>> +       y = mode->vdisplay;
>>> +
>>> +       /* Hisilicon has to set up a new register for PLL control
>>> +        *(CRT_PLL1_HS & CRT_PLL2_HS).
>>> +        */
>>> +       if (x == 800 && y == 600) {
>>> +               pll1 = CRT_PLL1_HS_40MHZ;
>>> +               pll2 = CRT_PLL2_HS_40MHZ;
>>> +       } else if (x == 1024 && y == 768) {
>>> +               pll1 = CRT_PLL1_HS_65MHZ;
>>> +               pll2 = CRT_PLL2_HS_65MHZ;
>>> +       } else if (x == 1152 && y == 864) {
>>> +               pll1 = CRT_PLL1_HS_80MHZ_1152;
>>> +               pll2 = CRT_PLL2_HS_80MHZ;
>>> +       } else if (x == 1280 && y == 768) {
>>> +               pll1 = CRT_PLL1_HS_80MHZ;
>>> +               pll2 = CRT_PLL2_HS_80MHZ;
>>> +       } else if (x == 1280 && y == 720) {
>>> +               pll1 = CRT_PLL1_HS_74MHZ;
>>> +               pll2 = CRT_PLL2_HS_74MHZ;
>>> +       } else if (x == 1280 && y == 960) {
>>> +               pll1 = CRT_PLL1_HS_108MHZ;
>>> +               pll2 = CRT_PLL2_HS_108MHZ;
>>> +       } else if (x == 1280 && y == 1024) {
>>> +               pll1 = CRT_PLL1_HS_108MHZ;
>>> +               pll2 = CRT_PLL2_HS_108MHZ;
>>> +       } else if (x == 1600 && y == 1200) {
>>> +               pll1 = CRT_PLL1_HS_162MHZ;
>>> +               pll2 = CRT_PLL2_HS_162MHZ;
>>> +       } else if (x == 1920 && y == 1080) {
>>> +               pll1 = CRT_PLL1_HS_148MHZ;
>>> +               pll2 = CRT_PLL2_HS_148MHZ;
>>> +       } else if (x == 1920 && y == 1200) {
>>> +               pll1 = CRT_PLL1_HS_193MHZ;
>>> +               pll2 = CRT_PLL2_HS_193MHZ;
>>> +       } else /* default to VGA clock */ {
>>> +               pll1 = CRT_PLL1_HS_25MHZ;
>>> +               pll2 = CRT_PLL2_HS_25MHZ;
>>> +       }
>>
>>
>> This seems like something that should be checked in atomic_check so
>> you can be sure the mode is supported.
>>
>> It would also be nice to pull this out into a separate function (and a
>> lookup table if you're feeling adventurous)
>
>
> a lookup table seems good, thanks.
>
>
>>
>>> +
>>> +       writel(pll2, hidev->mmio + CRT_PLL2_HS);
>>> +       set_vclock_hisilicon(dev, pll1);
>>> +
>>> +       /* Hisilicon has to set up the top-left and bottom-right
>>> +        * registers as well.
>>> +        * Note that normal chip only use those two register for
>>> +        * auto-centering mode.
>>> +        */
>>> +       writel((HIBMC_CRT_AUTO_CENTERING_TL_TOP(0) &
>>> +               HIBMC_CRT_AUTO_CENTERING_TL_TOP_MSK) |
>>> +              (HIBMC_CRT_AUTO_CENTERING_TL_LEFT(0) &
>>> +               HIBMC_CRT_AUTO_CENTERING_TL_LEFT_MSK),
>>> +              hidev->mmio + HIBMC_CRT_AUTO_CENTERING_TL);
>>> +
>>> +       writel((HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM(y - 1) &
>>> +               HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
>>> +              (HIBMC_CRT_AUTO_CENTERING_BR_RIGHT(x - 1) &
>>> +               HIBMC_CRT_AUTO_CENTERING_BR_RIGHT_MASK),
>>> +               hidev->mmio + HIBMC_CRT_AUTO_CENTERING_BR);
>>> +
>>> +       /* Assume common fields in ctrl have been properly set before
>>> +        * calling this function.
>>> +        * This function only sets the extra fields in ctrl.
>>> +        */
>>> +
>>> +       /* Set bit 25 of display controller: Select CRT or VGA clock */
>>> +       ctrl &= ~HIBMC_CRT_DISP_CTL_CRTSELECT_MASK;
>>> +       ctrl &= ~HIBMC_CRT_DISP_CTL_CLOCK_PHASE_MASK;
>>> +
>>> +       ctrl |= HIBMC_CRT_DISP_CTL_CRTSELECT(CRTSELECT_CRT);
>>> +
>>> +       /*ctrl = FIELD_SET(ctrl, HIBMC_CRT_DISP_CTL, CRTSELECT, CRT);*/
>>
>>
>> What's the deal with this commented code?
>
>
> sorry, will clean up.
>
>>
>>> +
>>> +       /* Set bit 14 of display controller */
>>> +       /*ctrl &= FIELD_CLEAR(HIBMC_CRT_DISP_CTL, CLOCK_PHASE);*/
>>> +
>>> +       /* clock_phase_polarity is 0 */
>>> +       ctrl |= HIBMC_CRT_DISP_CTL_CLOCK_PHASE(PHASE_ACTIVE_HIGH);
>>> +       /*ctrl = FIELD_SET(ctrl, HIBMC_CRT_DISP_CTL,*/
>>> +       /*CLOCK_PHASE, ACTIVE_HIGH);*/
>>
>>
>> Here too...
>
>
> ditto.
>
>>
>>> +
>>> +       writel(ctrl, hidev->mmio + HIBMC_CRT_DISP_CTL);
>>> +
>>> +       return ctrl;
>>> +}
>>> +
>>> +static void hibmc_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>> +{
>>> +       unsigned int val;
>>> +       struct drm_display_mode *mode = &crtc->state->mode;
>>> +       struct drm_device *dev = crtc->dev;
>>> +       struct hibmc_drm_device *hidev = dev->dev_private;
>>> +
>>> +       writel(format_pll_reg(), hidev->mmio + HIBMC_CRT_PLL_CTRL);
>>> +       writel((HIBMC_CRT_HORZ_TOTAL_TOTAL(mode->htotal - 1) &
>>> +               HIBMC_CRT_HORZ_TOTAL_TOTAL_MASK) |
>>> +               (HIBMC_CRT_HORZ_TOTAL_DISPLAY_END(mode->hdisplay - 1) &
>>> +               HIBMC_CRT_HORZ_TOTAL_DISPLAY_END_MASK),
>>
>>
>> You could probably macroize this code to make it more readable
>
>
>
>         #define HIBMC_FIELD(field, value) (field(value) & filed##_MASK)
>
>         writel(HIBMC_FIELD(HIBMC_CRT_HORZ_TOTAL_TOTAL, mode->htotal - 1) |
>                HIBMC_FIELD(HIBMC_CRT_HORZ_TOTAL_DISPLAY_END, mode->hdisplay
> - 1),
>                hidev->mmio + HIBMC_CRT_HORZ_TOTAL);
>
> Is above ok?
>

Seems like an improvement.

>
>
>
>>
>>> +               hidev->mmio + HIBMC_CRT_HORZ_TOTAL);
>>> +
>>> +       writel((HIBMC_CRT_HORZ_SYNC_WIDTH(mode->hsync_end -
>>> mode->hsync_start)
>>> +               & HIBMC_CRT_HORZ_SYNC_WIDTH_MASK) |
>>> +               (HIBMC_CRT_HORZ_SYNC_START(mode->hsync_start - 1)
>>> +               & HIBMC_CRT_HORZ_SYNC_START_MASK),
>>> +               hidev->mmio + HIBMC_CRT_HORZ_SYNC);
>>> +
>>> +       writel((HIBMC_CRT_VERT_TOTAL_TOTAL(mode->vtotal - 1) &
>>> +               HIBMC_CRT_VERT_TOTAL_TOTAL_MASK) |
>>> +               (HIBMC_CRT_VERT_TOTAL_DISPLAY_END(mode->vdisplay - 1) &
>>> +               HIBMC_CRT_VERT_TOTAL_DISPLAY_END_MASK),
>>> +               hidev->mmio + HIBMC_CRT_VERT_TOTAL);
>>> +
>>> +       writel((HIBMC_CRT_VERT_SYNC_HEIGHT(mode->vsync_end -
>>> mode->vsync_start)
>>> +               & HIBMC_CRT_VERT_SYNC_HEIGHT_MASK) |
>>> +              (HIBMC_CRT_VERT_SYNC_START(mode->vsync_start - 1) &
>>> +               HIBMC_CRT_VERT_SYNC_START_MASK),
>>> +               hidev->mmio + HIBMC_CRT_VERT_SYNC);
>>> +
>>> +       val = HIBMC_CRT_DISP_CTL_VSYNC_PHASE(0) &
>>> +             HIBMC_CRT_DISP_CTL_VSYNC_PHASE_MASK;
>>> +       val |= HIBMC_CRT_DISP_CTL_HSYNC_PHASE(0) &
>>> +              HIBMC_CRT_DISP_CTL_HSYNC_PHASE_MASK;
>>> +       val |= HIBMC_CRT_DISP_CTL_TIMING(ENABLE);
>>> +       val |= HIBMC_CRT_DISP_CTL_PLANE(ENABLE);
>>> +
>>> +       display_ctrl_adjust(dev, mode, val);
>>> +}
>>> +
>>> +static void hibmc_crtc_atomic_begin(struct drm_crtc *crtc,
>>> +                                   struct drm_crtc_state *old_state)
>>> +{
>>> +       unsigned int reg;
>>> +       struct drm_device *dev = crtc->dev;
>>> +       struct hibmc_drm_device *hidev = dev->dev_private;
>>> +
>>> +       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);
>>> +
>>> +       /* We can add more initialization as needed. */
>>> +}
>>> +
>>> +static void hibmc_crtc_atomic_flush(struct drm_crtc *crtc,
>>> +                                   struct drm_crtc_state *old_state)
>>> +
>>> +{
>>> +       unsigned long flags;
>>> +
>>> +       spin_lock_irqsave(&crtc->dev->event_lock, flags);
>>> +       if (crtc->state->event)
>>> +               drm_crtc_send_vblank_event(crtc, crtc->state->event);
>>> +       crtc->state->event = NULL;
>>> +
>>> +       spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>>> +}
>>> +
>>> +/* These provide the minimum set of functions required to handle a CRTC
>>> */
>>
>>
>> nit: don't need this comment
>
>
> will delete, thanks.
>
>
>>
>>> +static const struct drm_crtc_funcs hibmc_crtc_funcs = {
>>> +       .page_flip = drm_atomic_helper_page_flip,
>>> +       .set_config = drm_atomic_helper_set_config,
>>> +       .destroy = drm_crtc_cleanup,
>>> +       .reset = drm_atomic_helper_crtc_reset,
>>> +       .atomic_duplicate_state =
>>> drm_atomic_helper_crtc_duplicate_state,
>>> +       .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>> +};
>>> +
>>> +static const struct drm_crtc_helper_funcs hibmc_crtc_helper_funcs = {
>>> +       .enable         = hibmc_crtc_enable,
>>> +       .disable        = hibmc_crtc_disable,
>>> +       .mode_set_nofb  = hibmc_crtc_mode_set_nofb,
>>> +       .atomic_check   = hibmc_crtc_atomic_check,
>>> +       .atomic_begin   = hibmc_crtc_atomic_begin,
>>> +       .atomic_flush   = hibmc_crtc_atomic_flush,
>>> +};
>>> +
>>> +int hibmc_crtc_init(struct hibmc_drm_device *hidev)
>>> +{
>>> +       struct drm_device *dev = hidev->dev;
>>> +       struct drm_crtc *crtc = &hidev->crtc;
>>> +       struct drm_plane *plane = &hidev->plane;
>>> +       int ret;
>>> +
>>> +       ret = drm_crtc_init_with_planes(dev, crtc, plane,
>>> +                                       NULL, &hibmc_crtc_funcs, NULL);
>>> +       if (ret) {
>>> +               DRM_ERROR("failed to init crtc.\n");
>>
>>
>> print return code
>
>
> agreed, thanks.
>
>>
>>> +               return ret;
>>> +       }
>>> +
>>> +       drm_mode_crtc_set_gamma_size(crtc, 256);
>>
>>
>> check return code
>
>
> agreed though none of other drivers has done this,
> thanks.
>
>>
>>> +       drm_crtc_helper_add(crtc, &hibmc_crtc_helper_funcs);
>>> +       return 0;
>>> +}
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> index 7d96583..303cd36 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>>> @@ -119,6 +119,12 @@ static int hibmc_kms_init(struct hibmc_drm_device
>>> *hidev)
>>>                  return ret;
>>>          }
>>>
>>> +       ret = hibmc_crtc_init(hidev);
>>> +       if (ret) {
>>> +               DRM_ERROR("failed to init crtc.\n");
>>> +               return ret;
>>> +       }
>>
>>
>> Typically the plane is initialized internally in the crtc driver. I
>> think this is a good design pattern, and you should probably use it.
>>
>> So how about squashing this down with the plane patch and keeping the
>> plane inside hibmc_drm_de.c?
>
>
> understood after i looked at intel_display.c, this file will be merged
> with patch 4/9, and the tile will be: 'drm/hisilicon/hibmc: Add display
> engine'.
>

Great, thanks.

>>
>>> +
>>>          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 49e39d2..5731ec2 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> @@ -46,6 +46,7 @@ struct hibmc_drm_device {
>>>          /* drm */
>>>          struct drm_device  *dev;
>>>          struct drm_plane plane;
>>
>>
>> I don't think you should be keeping track of plane here. plane is only
>> used in the crtc init, which should be addressed by the previous
>> comment.
>
>
> so allocate with devm_kzalloc(sizeof(*plane)) and remove it from
> hibmc_drm_device?
>
>>
>>
>>> +       struct drm_crtc crtc;
>>
>>
>> crtc is only used in the irq handler, so you could remove this here
>> and just call drm_handle_vblank(dev, 0) in the handler.
>
>
> so allocate with devm_kzalloc(sizeof(*crtc)) and remove it from
> hibmc_drm_device, when driver unload drm_crtc_cleanup() will be
> called and finally memory will be freed before quit.
>

Yep, precisely.

Sean

>>
>>
>>>          bool mode_config_initialized;
>>>
>>>          /* ttm */
>>> @@ -85,6 +86,7 @@ static inline struct hibmc_bo *gem_to_hibmc_bo(struct
>>> drm_gem_object *gem)
>>>   #define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
>>>
>>>   int hibmc_plane_init(struct hibmc_drm_device *hidev);
>>> +int hibmc_crtc_init(struct hibmc_drm_device *hidev);
>>>   int hibmc_fbdev_init(struct hibmc_drm_device *hidev);
>>>   void hibmc_fbdev_fini(struct hibmc_drm_device *hidev);
>>>
>>> --
>>> 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
>>
>> .
>>
>
>
> --
> Regards, Rongrong
>
>
> _______________________________________________
> 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 v5 1/3] dt-bindings: mediatek: Add a binding for Mediatek JPEG Decoder
From: Rob Herring @ 2016-11-14 17:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478586880-3923-2-git-send-email-rick.chang@mediatek.com>

On Tue, Nov 08, 2016 at 02:34:38PM +0800, Rick Chang wrote:
> Add a DT binding documentation for Mediatek JPEG Decoder of
> MT2701 SoC.
> 
> Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> ---
>  .../bindings/media/mediatek-jpeg-decoder.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.txt

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [RESEND][PATCH 1/6] dt-bindings: arm: Update bindings for LS2088A targets
From: Rob Herring @ 2016-11-14 17:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478597664-14799-2-git-send-email-abhimanyu.saini@nxp.com>

On Tue, Nov 08, 2016 at 03:04:19PM +0530, Abhimanyu Saini wrote:
> Add compatible strings for LS2088A RDB and QDS board.
> 
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.kumar@nxp.com>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.txt | 7 +++++++
>  1 file changed, 7 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [RESEND][PATCH 2/6] dt-bindings: pci: Update bindings for LS2088A
From: Rob Herring @ 2016-11-14 17:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478597664-14799-3-git-send-email-abhimanyu.saini@nxp.com>

On Tue, Nov 08, 2016 at 03:04:20PM +0530, Abhimanyu Saini wrote:
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.kumar@nxp.com>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/layerscape-pci.txt | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [RESEND][PATCH 3/6] dt-bindings: mtd: fsl-quadspi: Update qspi bindings for LS2088A
From: Rob Herring @ 2016-11-14 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478597664-14799-4-git-send-email-abhimanyu.saini@nxp.com>

On Tue, Nov 08, 2016 at 03:04:21PM +0530, Abhimanyu Saini wrote:
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.kumar@nxp.com>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> ---
>  Documentation/devicetree/bindings/mtd/fsl-quadspi.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [RESEND][PATCH 4/6] dt-bindings: spi: Update dspi bindings for LS2088A
From: Rob Herring @ 2016-11-14 17:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478597664-14799-5-git-send-email-abhimanyu.saini@nxp.com>

On Tue, Nov 08, 2016 at 03:04:22PM +0530, Abhimanyu Saini wrote:
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.kumar@nxp.com>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [RESEND][PATCH 5/6] dt-bindings: gpio: Update gpio bindings for LS2088A
From: Rob Herring @ 2016-11-14 17:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478597664-14799-6-git-send-email-abhimanyu.saini@nxp.com>

On Tue, Nov 08, 2016 at 03:04:23PM +0530, Abhimanyu Saini wrote:
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.kumar@nxp.com>
> Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 1/2] mfd: pm8921: add support to pm8821
From: Rob Herring @ 2016-11-14 17:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478622577-20699-1-git-send-email-srinivas.kandagatla@linaro.org>

On Tue, Nov 08, 2016 at 04:29:36PM +0000, Srinivas Kandagatla wrote:
> This patch adds support to PM8821 PMIC and interrupt support.
> PM8821 is companion device that supplements primary PMIC PM8921 IC.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> Tested this patch for MPP and IRQ functionality on IFC6410 and SD600 EVAL
> board with mpps PM8821 and PM8921.
> 
>  .../devicetree/bindings/mfd/qcom-pm8xxx.txt        |   1 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/mfd/pm8921-core.c                          | 368 +++++++++++++++++++--
>  2 files changed, 340 insertions(+), 29 deletions(-)

^ permalink raw reply

* [PATCH fpga 5/9] fpga zynq: Remove priv->dev
From: Moritz Fischer @ 2016-11-14 17:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.10.1611140913280.3739@atull-VirtualBox>

Hi Jason,

this one could be independent of the other patches, this is cleanup and
could be merged before the rest (that's still in discussion) if you pull it out.

On Mon, Nov 14, 2016 at 7:13 AM, atull <atull@opensource.altera.com> wrote:
> On Wed, 9 Nov 2016, Jason Gunthorpe wrote:
>
> Hi Jason,
>
>     Acked-by: Alan Tull <atull@opensource.altera.com>
>
> Alan
>
>> socfpga uses mgr->dev for debug prints, there should be consistency
>> here, so standardize on that. The only other use was for dma
>> which can be replaced with mgr->dev.parent.
>>
>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Modulo the comments above:

Acked-by: Moritz Fischer <moritz.fischer@ettus.com>
>> ---
>>  drivers/fpga/zynq-fpga.c | 22 ++++++++++------------
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c
>> index 3ffc5fcc3072..ac2deae92dbd 100644
>> --- a/drivers/fpga/zynq-fpga.c
>> +++ b/drivers/fpga/zynq-fpga.c
>> @@ -118,7 +118,6 @@
>>  #define FPGA_RST_NONE_MASK           0x0
>>
>>  struct zynq_fpga_priv {
>> -     struct device *dev;
>>       int irq;
>>       struct clk *clk;
>>
>> @@ -188,7 +187,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>        * least the sync word and something else to do anything.
>>        */
>>       if (count <= 4 || (count % 4) != 0) {
>> -             dev_err(priv->dev,
>> +             dev_err(&mgr->dev,
>>                       "Invalid bitstream size, must be multiples of 4 bytes\n");
>>               return -EINVAL;
>>       }
>> @@ -200,7 +199,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>       /* don't globally reset PL if we're doing partial reconfig */
>>       if (!(flags & FPGA_MGR_PARTIAL_RECONFIG)) {
>>               if (!zynq_fpga_has_sync(buf, count)) {
>> -                     dev_err(priv->dev,
>> +                     dev_err(&mgr->dev,
>>                               "Invalid bitstream, could not find a sync word. Bitstream must be a byte swaped .bin file\n");
>>                       err = -EINVAL;
>>                       goto out_err;
>> @@ -233,7 +232,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>                                            INIT_POLL_DELAY,
>>                                            INIT_POLL_TIMEOUT);
>>               if (err) {
>> -                     dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n");
>> +                     dev_err(&mgr->dev, "Timeout waiting for PCFG_INIT\n");
>>                       goto out_err;
>>               }
>>
>> @@ -247,7 +246,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>                                            INIT_POLL_DELAY,
>>                                            INIT_POLL_TIMEOUT);
>>               if (err) {
>> -                     dev_err(priv->dev, "Timeout waiting for !PCFG_INIT\n");
>> +                     dev_err(&mgr->dev, "Timeout waiting for !PCFG_INIT\n");
>>                       goto out_err;
>>               }
>>
>> @@ -261,7 +260,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>                                            INIT_POLL_DELAY,
>>                                            INIT_POLL_TIMEOUT);
>>               if (err) {
>> -                     dev_err(priv->dev, "Timeout waiting for PCFG_INIT\n");
>> +                     dev_err(&mgr->dev, "Timeout waiting for PCFG_INIT\n");
>>                       goto out_err;
>>               }
>>       }
>> @@ -278,7 +277,7 @@ static int zynq_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags,
>>       /* check that we have room in the command queue */
>>       status = zynq_fpga_read(priv, STATUS_OFFSET);
>>       if (status & STATUS_DMA_Q_F) {
>> -             dev_err(priv->dev, "DMA command queue full\n");
>> +             dev_err(&mgr->dev, "DMA command queue full\n");
>>               err = -EBUSY;
>>               goto out_err;
>>       }
>> @@ -309,7 +308,8 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
>>
>>       priv = mgr->priv;
>>
>> -     kbuf = dma_alloc_coherent(priv->dev, count, &dma_addr, GFP_KERNEL);
>> +     kbuf =
>> +         dma_alloc_coherent(mgr->dev.parent, count, &dma_addr, GFP_KERNEL);
>>       if (!kbuf)
>>               return -ENOMEM;
>>
>> @@ -356,7 +356,7 @@ static int zynq_fpga_ops_write(struct fpga_manager *mgr,
>>       goto out_clk;
>>
>>  out_report:
>> -     dev_err(priv->dev,
>> +     dev_err(&mgr->dev,
>>               "%s: INT_STS:0x%x CTRL:0x%x LOCK:0x%x INT_MASK:0x%x STATUS:0x%x MCTRL:0x%x\n",
>>               why,
>>               intr_status,
>> @@ -368,7 +368,7 @@ out_report:
>>  out_clk:
>>       clk_disable(priv->clk);
>>  out_free:
>> -     dma_free_coherent(priv->dev, count, kbuf, dma_addr);
>> +     dma_free_coherent(mgr->dev.parent, count, kbuf, dma_addr);
>>       return err;
>>  }
>>
>> @@ -445,8 +445,6 @@ static int zynq_fpga_probe(struct platform_device *pdev)
>>       if (!priv)
>>               return -ENOMEM;
>>
>> -     priv->dev = dev;
>> -
>>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       priv->io_base = devm_ioremap_resource(dev, res);
>>       if (IS_ERR(priv->io_base))
>> --
>> 2.1.4
>>
>>
Cheers,

Moritz

^ permalink raw reply

* [PATCH 1/6] dt-bindings: mdio-mux: Add documentation for mdio mux for NSP SoC
From: Rob Herring @ 2016-11-14 17:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478683994-12008-2-git-send-email-yendapally.reddy@broadcom.com>

On Wed, Nov 09, 2016 at 04:33:09AM -0500, Yendapally Reddy Dhananjaya Reddy wrote:
> Add documentation for mdio mux available in Broadcom NSP SoC
> 
> Signed-off-by: Yendapally Reddy Dhananjaya Reddy <yendapally.reddy@broadcom.com>
> ---
>  .../devicetree/bindings/net/brcm,mdio-mux-nsp.txt  | 57 ++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/brcm,mdio-mux-nsp.txt

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 2/6] dt-bindings: phy: Add documentation for NSP USB3 PHY
From: Rob Herring @ 2016-11-14 17:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478683994-12008-3-git-send-email-yendapally.reddy@broadcom.com>

On Wed, Nov 09, 2016 at 04:33:10AM -0500, Yendapally Reddy Dhananjaya Reddy wrote:
> Add documentation for USB3 PHY available in Northstar plus SoC
> 
> Signed-off-by: Yendapally Reddy Dhananjaya Reddy <yendapally.reddy@broadcom.com>
> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> new file mode 100644
> index 0000000..30cf4b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> @@ -0,0 +1,39 @@
> +Broadcom USB3 phy binding northstar plus SoC
> +This is a child bus node of "brcm,mdio-mux-nsp" node.
> +
> +Required mdio bus properties:
> +- reg: MDIO Bus number for the MDIO interface
> +- #address-cells: must be 1
> +- #size-cells: must be 0
> +
> +Required PHY properties:
> +- compatible: should be "brcm,nsp-usb3-phy"
> +- reg: Phy address in the MDIO interface
> +- usb3-ctrl-syscon: handler of syscon node defining physical address
> +  of usb3 control register.
> +- #phy-cells: must be 0
> +
> +Required usb3 control properties:
> +- compatible: should be "brcm,nsp-usb3-ctrl"
> +- reg: offset and length of the control registers
> +
> +Example:
> +
> +	mdio at 0 {
> +		reg = <0x0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		usb3_phy: usb3-phy at 10 {

Just 'usb-phy at 10'. With that,

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 3/5] net: thunderx: Fix configuration of L3/L4 length checking
From: Sunil Kovvuri @ 2016-11-14 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114123350.GA2449@Red>

>>You could use the BIT() macro here
Thanks, will change and resubmit.

Sunil.

^ permalink raw reply

* [PATCH v2 7/9] arm64: dts: rockchip: add pd_edp node for rk3399
From: Doug Anderson @ 2016-11-14 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478697721-2323-8-git-send-email-wxt@rock-chips.com>

Caesar,

On Wed, Nov 9, 2016 at 5:21 AM, Caesar Wang <wxt@rock-chips.com> wrote:
> From: zhangqing <zhangqing@rock-chips.com>
>
> 1. add pd node for RK3399 Soc
> 2. create power domain tree
> 3. add qos node for domain

No step #3 since there doesn't appear to be a qos node for eDP.  Your
patch doesn't add one and I can't find one in the TRM.

> 4. add the pd support for edp
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
> Changes in v2: None
>
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 74deb44..09ebf4e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -835,6 +835,10 @@
>                         };
>
>                         /* These power domains are grouped by VD_LOGIC */
> +                       pd_edp at RK3399_PD_EDP {
> +                               reg = <RK3399_PD_EDP>;
> +                               clocks = <&cru PCLK_EDP_CTRL>;

Are you sure that PCLK_EDP isn't needed as well?  After the super
hard-to-debug problems we just faced with the missing GMAC clock in
the power domains, I figure it's at least worth a check.  ;)

> +                       };
>                         pd_emmc at RK3399_PD_EMMC {
>                                 reg = <RK3399_PD_EMMC>;
>                                 clocks = <&cru ACLK_EMMC>;
> @@ -1364,6 +1368,7 @@
>                 status = "disabled";
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&edp_hpd>;
> +               power-domains = <&power RK3399_PD_EDP>;
>
>                 ports {
>                         #address-cells = <1>;

Other than the question about the clock and the nits about the commit
message, this all looks fine to me.  Feel free to add my Reviewed-by
if you fix those things.


-Doug

^ permalink raw reply

* [PATCH 1/4] fpga mgr: Introduce FPGA capabilities
From: Moritz Fischer @ 2016-11-14 17:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.10.1611140801480.2786@atull-VirtualBox>

Hi Alan,

On Mon, Nov 14, 2016 at 6:06 AM, atull <atull@opensource.altera.com> wrote:
> On Mon, 7 Nov 2016, Moritz Fischer wrote:
>
>> Add FPGA capabilities as a way to express the capabilities
>> of a given FPGA manager.
>>
>> Removes code duplication by comparing the low-level driver's
>> capabilities at the framework level rather than having each driver
>> check for supported operations in the write_init() callback.
>>
>> This allows for extending with additional capabilities, similar
>> to the the dmaengine framework's implementation.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> Cc: Alan Tull <atull@opensource.altera.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>> Cc: linux-kernel at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org
>> ---
>>
>> Changes from RFC:
>> * in the RFC the caps weren't actually stored into the struct fpga_mgr
>>
>> Note:
>>
>> If people disagree on the typedef being a 'false positive' I can fix
>> that in a future rev of the patchset.
>>
>> Thanks,
>>
>>     Moritz
>>
>> ---
>>  drivers/fpga/fpga-mgr.c       | 15 ++++++++++++++
>>  drivers/fpga/socfpga.c        | 10 +++++-----
>>  drivers/fpga/zynq-fpga.c      |  7 ++++++-
>>  include/linux/fpga/fpga-mgr.h | 46 ++++++++++++++++++++++++++++++++++++++++++-
>>  4 files changed, 71 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
>> index 953dc91..ed57c17 100644
>> --- a/drivers/fpga/fpga-mgr.c
>> +++ b/drivers/fpga/fpga-mgr.c
>> @@ -49,6 +49,18 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>>       struct device *dev = &mgr->dev;
>>       int ret;
>>
>> +     if (flags & FPGA_MGR_PARTIAL_RECONFIG &&
>> +         !fpga_mgr_has_cap(FPGA_MGR_CAP_PARTIAL_RECONF, mgr->caps)) {
>> +             dev_err(dev, "Partial reconfiguration not supported\n");
>> +             return -ENOTSUPP;
>> +     }
>> +
>> +     if (flags & FPGA_MGR_FULL_RECONFIG &&
>> +         !fpga_mgr_has_cap(FPGA_MGR_CAP_FULL_RECONF, mgr->caps)) {
>> +             dev_err(dev, "Full reconfiguration not supported\n");
>> +             return -ENOTSUPP;
>> +     }
>> +
>
> Could you move the checks to their own function like
> 'fpga_mgr_check_caps()' or something?  I really like it if we can keep
> the functions short, like a screen or so where it's practicle to do
> so and I could see the number of caps growing here.

Absolutely. Great suggestion.

> The only counter argument I could think of is if a cap affects the sequence
> in this function.  Hmmm...

Oh you mean the cap being there affecting the sequence in *this* function?
I'd suggest we address that when we run into a cap that requires this.

Cheers,

Moritz

^ permalink raw reply

* [PATCH 0/5] net: thunderx: Miscellaneous fixes
From: Sunil Kovvuri @ 2016-11-14 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2948c47d-0f15-8153-440b-7a2c753b7251@suse.com>

>>If so, please add "Cc: stable at vger.kernel.org" to the Sigend-off list.
Yes they are, thanks, will do that along with resubmission.

Sunil.

^ permalink raw reply

* [PATCH 0/3] ARM64: dts: meson-gxl: Enable Ethernet
From: Kevin Hilman @ 2016-11-14 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161107104357.24428-1-narmstrong@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> The Amlogic Meson GXL SoCs have an internal RMII PHY that is muxed with the
> external RGMII pins.
>
> The internal PHY is added in the GXL dtsi and support for each
> board is added in intermediate board family dtsi or final dts.

Tested external phy and internal phy (using p231 DT) on my p230 board.

Applied to v4.10/dt64

Kevin

^ permalink raw reply

* [PATCH v4 3/3] dt-bindings: i2c: pxa: Update the documentation for the Armada 3700
From: Rob Herring @ 2016-11-14 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161109115715.2557-4-romain.perier@free-electrons.com>

On Wed, Nov 09, 2016 at 12:57:15PM +0100, Romain Perier wrote:
> This commit documents the compatible string to have the compatibility for
> the I2C unit found in the Armada 3700.
> 
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
> ---
> 
> Changes in v2:
>  - Fixed wrong compatible string, it should be "marvell,armada-3700-i2c"
>    and not "marvell,armada-3700".
> 
>  Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox