Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sekhar Nori @ 2016-11-11 10:47 UTC (permalink / raw)
  To: Rob Herring, Bartosz Golaszewski
  Cc: Mark Rutland, linux-devicetree, David Airlie, Kevin Hilman,
	Michael Turquette, Russell King, linux-drm, LKML, Tomi Valkeinen,
	Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <20161109182430.yh6uqeu2ufzkctww@rob-hp-laptop>

On Wednesday 09 November 2016 11:54 PM, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 03:45:34PM +0100, Bartosz Golaszewski wrote:
>> Create a new driver for the da8xx DDR2/mDDR controller and implement
>> support for writing to the Peripheral Bus Burst Priority Register.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 +++
> 
> Acked-by: Rob Herring <robh@kernel.org>

Applied to v4.10/drivers

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: liviu.dudau @ 2016-11-11 10:37 UTC (permalink / raw)
  To: Gabriele Paoloni
  Cc: Yuanzhichang, catalin.marinas@arm.com, will.deacon@arm.com,
	robh+dt@kernel.org, bhelgaas@google.com, mark.rutland@arm.com,
	olof@lixom.net, arnd@arndb.de,
	linux-arm-kernel@lists.infradead.org, lorenzo.pieralisi@arm.com,
	linux-kernel@vger.kernel.org, Linuxarm,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-serial@vger.kernel.org,
	"minyard@acm.org" <mi>
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F8F5F06@lhreml507-mbx>

On Thu, Nov 10, 2016 at 04:06:40PM +0000, Gabriele Paoloni wrote:
> Hi Liviu
> 
> > -----Original Message-----
> > From: liviu.dudau@arm.com [mailto:liviu.dudau@arm.com]
> > Sent: 09 November 2016 16:51
> > To: Gabriele Paoloni
> > Cc: Yuanzhichang; catalin.marinas@arm.com; will.deacon@arm.com;
> > robh+dt@kernel.org; bhelgaas@google.com; mark.rutland@arm.com;
> > olof@lixom.net; arnd@arndb.de; linux-arm-kernel@lists.infradead.org;
> > lorenzo.pieralisi@arm.com; linux-kernel@vger.kernel.org; Linuxarm;
> > devicetree@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> > serial@vger.kernel.org; minyard@acm.org; benh@kernel.crashing.org;
> > zourongrong@gmail.com; John Garry; zhichang.yuan02@gmail.com;
> > kantyzc@163.com; xuwei (O)
> > Subject: Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for
> > special ISA
> > 
> > On Wed, Nov 09, 2016 at 04:16:17PM +0000, Gabriele Paoloni wrote:
> > > Hi Liviu
> > >
> > > Thanks for reviewing
> > >
> > 
> > [removed some irrelevant part of discussion, avoid crazy formatting]
> > 
> > > > > +/**
> > > > > + * addr_is_indirect_io - check whether the input taddr is for
> > > > indirectIO.
> > > > > + * @taddr: the io address to be checked.
> > > > > + *
> > > > > + * Returns 1 when taddr is in the range; otherwise return 0.
> > > > > + */
> > > > > +int addr_is_indirect_io(u64 taddr)
> > > > > +{
> > > > > +	if (arm64_extio_ops->start > taddr || arm64_extio_ops->end
> > <
> > > > taddr)
> > > >
> > > > start >= taddr ?
> > >
> > > Nope... if  (taddr < arm64_extio_ops->start || taddr >
> > arm64_extio_ops->end)
> > > then taddr is outside the range [start; end] and will return 0;
> > otherwise
> > > it will return 1...
> > 
> > Oops, sorry, did not pay attention to the returned value. The check is
> > correct as it is, no need to change then.
> > 
> > >
> > > >
> > > > > +		return 0;
> > > > > +
> > > > > +	return 1;
> > > > > +}
> > > > >
> > > > >  BUILD_EXTIO(b, u8)
> > > > >
> > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > > > index 02b2903..cc2a05d 100644
> > > > > --- a/drivers/of/address.c
> > > > > +++ b/drivers/of/address.c
> > > > > @@ -479,6 +479,50 @@ static int of_empty_ranges_quirk(struct
> > > > device_node *np)
> > > > >  	return false;
> > > > >  }
> > > > >
> > > > > +
> > > > > +/*
> > > > > + * of_isa_indirect_io - get the IO address from some isa reg
> > > > property value.
> > > > > + *	For some isa/lpc devices, no ranges property in ancestor
> > node.
> > > > > + *	The device addresses are described directly in their regs
> > > > property.
> > > > > + *	This fixup function will be called to get the IO address of
> > > > isa/lpc
> > > > > + *	devices when the normal of_translation failed.
> > > > > + *
> > > > > + * @parent:	points to the parent dts node;
> > > > > + * @bus:		points to the of_bus which can be used to parse
> > > > address;
> > > > > + * @addr:	the address from reg property;
> > > > > + * @na:		the address cell counter of @addr;
> > > > > + * @presult:	store the address paresed from @addr;
> > > > > + *
> > > > > + * return 1 when successfully get the I/O address;
> > > > > + * 0 will return for some failures.
> > > >
> > > > Bah, you are returning a signed int, why 0 for failure? Return a
> > > > negative value with
> > > > error codes. Otherwise change the return value into a bool.
> > >
> > > Yes we'll move to bool
> > >
> > > >
> > > > > + */
> > > > > +static int of_get_isa_indirect_io(struct device_node *parent,
> > > > > +				struct of_bus *bus, __be32 *addr,
> > > > > +				int na, u64 *presult)
> > > > > +{
> > > > > +	unsigned int flags;
> > > > > +	unsigned int rlen;
> > > > > +
> > > > > +	/* whether support indirectIO */
> > > > > +	if (!indirect_io_enabled())
> > > > > +		return 0;
> > > > > +
> > > > > +	if (!of_bus_isa_match(parent))
> > > > > +		return 0;
> > > > > +
> > > > > +	flags = bus->get_flags(addr);
> > > > > +	if (!(flags & IORESOURCE_IO))
> > > > > +		return 0;
> > > > > +
> > > > > +	/* there is ranges property, apply the normal translation
> > > > directly. */
> > > >
> > > > s/there is ranges/if we have a 'ranges'/
> > >
> > > Thanks for spotting this
> > >
> > > >
> > > > > +	if (of_get_property(parent, "ranges", &rlen))
> > > > > +		return 0;
> > > > > +
> > > > > +	*presult = of_read_number(addr + 1, na - 1);
> > > > > +	/* this fixup is only valid for specific I/O range. */
> > > > > +	return addr_is_indirect_io(*presult);
> > > > > +}
> > > > > +
> > > > >  static int of_translate_one(struct device_node *parent, struct
> > > > of_bus *bus,
> > > > >  			    struct of_bus *pbus, __be32 *addr,
> > > > >  			    int na, int ns, int pna, const char *rprop)
> > > > > @@ -595,6 +639,15 @@ static u64 __of_translate_address(struct
> > > > device_node *dev,
> > > > >  			result = of_read_number(addr, na);
> > > > >  			break;
> > > > >  		}
> > > > > +		/*
> > > > > +		 * For indirectIO device which has no ranges
> > property, get
> > > > > +		 * the address from reg directly.
> > > > > +		 */
> > > > > +		if (of_get_isa_indirect_io(dev, bus, addr, na,
> > &result)) {
> > > > > +			pr_debug("isa indirectIO matched(%s)..addr =
> > > > 0x%llx\n",
> > > > > +				of_node_full_name(dev), result);
> > > > > +			break;
> > > > > +		}
> > > > >
> > > > >  		/* Get new parent bus and counts */
> > > > >  		pbus = of_match_bus(parent);
> > > > > @@ -688,8 +741,9 @@ static int __of_address_to_resource(struct
> > > > device_node *dev,
> > > > >  	if (taddr == OF_BAD_ADDR)
> > > > >  		return -EINVAL;
> > > > >  	memset(r, 0, sizeof(struct resource));
> > > > > -	if (flags & IORESOURCE_IO) {
> > > > > +	if (flags & IORESOURCE_IO && taddr >= PCIBIOS_MIN_IO) {
> > > > >  		unsigned long port;
> > > > > +
> > > > >  		port = pci_address_to_pio(taddr);
> > > > >  		if (port == (unsigned long)-1)
> > > > >  			return -EINVAL;
> > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > > index ba34907..1a08511 100644
> > > > > --- a/drivers/pci/pci.c
> > > > > +++ b/drivers/pci/pci.c
> > > > > @@ -3263,7 +3263,7 @@ int __weak
> > pci_register_io_range(phys_addr_t
> > > > addr, resource_size_t size)
> > > > >
> > > > >  #ifdef PCI_IOBASE
> > > > >  	struct io_range *range;
> > > > > -	resource_size_t allocated_size = 0;
> > > > > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> > > > >
> > > > >  	/* check if the range hasn't been previously recorded */
> > > > >  	spin_lock(&io_range_lock);
> > > > > @@ -3312,7 +3312,7 @@ phys_addr_t pci_pio_to_address(unsigned
> > long
> > > > pio)
> > > > >
> > > > >  #ifdef PCI_IOBASE
> > > > >  	struct io_range *range;
> > > > > -	resource_size_t allocated_size = 0;
> > > > > +	resource_size_t allocated_size = PCIBIOS_MIN_IO;
> > > >
> > > > Have you checked that pci_pio_to_address still returns valid values
> > > > after this? I know that
> > > > you are trying to take into account PCIBIOS_MIN_IO limit when
> > > > allocating reserving the IO ranges,
> > > > but the values added in the io_range_list are still starting from
> > zero,
> > > > no from PCIBIOS_MIN_IO,
> > >
> > > I think you're wrong here as in pci_address_to_pio we have:
> > > +	resource_size_t offset = PCIBIOS_MIN_IO;
> > >
> > > This should be enough to guarantee that the PIOs start at
> > > PCIBIOS_MIN_IO...right?
> > 
> > I don't think you can guarantee that the pio value that gets passed
> > into
> > pci_pio_to_address() always comes from a previously returned value by
> > pci_address_to_pio(). Maybe you can add a check in pci_pio_to_address()
> 
> Maybe I am missing something...could you make an exampleof a case
> where an IO toke doesn’t come from pci_address_to_pio() ?

Don't know, maybe it is coming from some DT or platform data? I was just
asking for confirmation that no one has seen issues there, I'm not saying
I've seen it happening.

Best regards,
Liviu

> 
> Thanks
> 
> Gab
> 
> 
> > 
> > 	if (pio < PCIBIOS_MIN_IO)
> > 		return address;
> > 
> > to avoid adding more checks in the list_for_each_entry() loop.
> > 
> > Best regards,
> > Liviu
> > 
> > >
> > >
> > > > so the calculation of the address in this function could return
> > > > negative values casted to pci_addr_t.
> > > >
> > > > Maybe you want to adjust the range->start value in
> > > > pci_register_io_range() as well to have it
> > > > offset by PCIBIOS_MIN_IO as well.
> > > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > > >
> > > > >  	if (pio > IO_SPACE_LIMIT)
> > > > >  		return address;
> > > > > @@ -3335,7 +3335,7 @@ unsigned long __weak
> > > > pci_address_to_pio(phys_addr_t address)
> > > > >  {
> > > > >  #ifdef PCI_IOBASE
> > > > >  	struct io_range *res;
> > > > > -	resource_size_t offset = 0;
> > > > > +	resource_size_t offset = PCIBIOS_MIN_IO;
> > > > >  	unsigned long addr = -1;
> > > > >
> > > > >  	spin_lock(&io_range_lock);
> > > > > diff --git a/include/linux/of_address.h
> > b/include/linux/of_address.h
> > > > > index 3786473..deec469 100644
> > > > > --- a/include/linux/of_address.h
> > > > > +++ b/include/linux/of_address.h
> > > > > @@ -24,6 +24,23 @@ struct of_pci_range {
> > > > >  #define for_each_of_pci_range(parser, range) \
> > > > >  	for (; of_pci_range_parser_one(parser, range);)
> > > > >
> > > > > +
> > > > > +#ifndef indirect_io_enabled
> > > > > +#define indirect_io_enabled indirect_io_enabled
> > > > > +static inline bool indirect_io_enabled(void)
> > > > > +{
> > > > > +	return false;
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > +#ifndef addr_is_indirect_io
> > > > > +#define addr_is_indirect_io addr_is_indirect_io
> > > > > +static inline int addr_is_indirect_io(u64 taddr)
> > > > > +{
> > > > > +	return 0;
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > >  /* Translate a DMA address from device space to CPU space */
> > > > >  extern u64 of_translate_dma_address(struct device_node *dev,
> > > > >  				    const __be32 *in_addr);
> > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > > > index 0e49f70..7f6bbb6 100644
> > > > > --- a/include/linux/pci.h
> > > > > +++ b/include/linux/pci.h
> > > > > @@ -2130,4 +2130,12 @@ static inline bool pci_ari_enabled(struct
> > > > pci_bus *bus)
> > > > >  /* provide the legacy pci_dma_* API */
> > > > >  #include <linux/pci-dma-compat.h>
> > > > >
> > > > > +/*
> > > > > + * define this macro here to refrain from compilation error for
> > some
> > > > > + * platforms. Please keep this macro at the end of this header
> > file.
> > > > > + */
> > > > > +#ifndef PCIBIOS_MIN_IO
> > > > > +#define PCIBIOS_MIN_IO		0
> > > > > +#endif
> > > > > +
> > > > >  #endif /* LINUX_PCI_H */
> > > > > --
> > > > > 1.9.1
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-
> > pci"
> > > > in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-
> > info.html
> > 
> > --
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCH V2 05/10] mfd: da9061: MFD core support
From: Lee Jones @ 2016-11-11 10:37 UTC (permalink / raw)
  To: Steve Twiss
  Cc: LINUX-KERNEL, DEVICETREE, Dmitry Torokhov, Eduardo Valentin,
	Guenter Roeck, LINUX-INPUT, LINUX-PM, LINUX-WATCHDOG,
	Liam Girdwood, Mark Brown, Mark Rutland, Rob Herring,
	Support Opensource, Wim Van Sebroeck, Zhang Rui
In-Reply-To: <bf02595c2fa32ba0c4311e2d480321b811fc9cf5.1477501000.git.stwiss.opensource@diasemi.com>

On Wed, 26 Oct 2016, Steve Twiss wrote:

> From: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> MFD support for DA9061 is provided as part of the DA9062 device driver.
> 
> The registers header file adds two new chip variant IDs defined in DA9061
> and DA9062 hardware. The core header file adds new software enumerations
> for listing the valid DA9061 IRQs and a da9062_compatible_types enumeration
> for distinguishing between DA9061/62 devices in software.
> 
> The core source code adds a new .compatible of_device_id entry. This is
> extended from DA9062 to support both "dlg,da9061" and "dlg,da9062". The
> .data entry now holds a reference to the enumerated device type.
> 
> A new regmap_irq_chip model is added for DA9061 and this supports the new
> list of regmap_irq entries. A new mfd_cell da9061_devs[] array lists the
> new sub system components for DA9061. Support is added for a new DA9061
> regmap_config which lists the correct readable, writable and volatile
> ranges for this chip.
> 
> The probe function uses the device tree compatible string to switch on the
> da9062_compatible_types and configure the correct mfd cells, irq chip and
> regmap config.
>  
> Kconfig is updated to reflect support for DA9061 and DA9062 PMICs.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> ---
> This patch applies against linux-next and v4.8
> 
> v1 -> v2
>  - Patch renamed from [PATCH V1 01/10] to [PATCH V2 05/10] -- these
>    changes were made to fix checkpatch warnings caused by the patch
>    set dependency order
>  - Fixed typo in the commit message "readble" to "readable"
>  - Removed the explicit cross-check to decide if there is a conflict
>    between the device tree compatible string and the hardware definition.
>    This patch assumes the device tree is correctly written and therefore
>    removes the need for a hardware/DT sanity check.
>  - Removed extra semicolon in drivers/mfd/da9062-core.c:877
>  - Re-write compatible entries into numerical order
> 
> Lee,
> 
> This patch adds support for the DA9061 PMIC. This is done as part of the
> existing DA9062 device driver by extending the of_device_id match table.
> This in turn allows new MFD cells, irq chip and regmap definitions to
> support DA9061.
> 
> Regards,
> Steve Twiss, Dialog Semiconductor Ltd.
> 
> 
>  drivers/mfd/Kconfig                  |   5 +-
>  drivers/mfd/da9062-core.c            | 427 +++++++++++++++++++++++++++++++++--
>  include/linux/mfd/da9062/core.h      |  27 ++-
>  include/linux/mfd/da9062/registers.h |   2 +
>  4 files changed, 441 insertions(+), 20 deletions(-)

[...]

> +static struct resource da9061_core_resources[] = {
> +	DEFINE_RES_NAMED(DA9061_IRQ_VDD_WARN, 1, "VDD_WARN", IORESOURCE_IRQ),
> +};
> +
> +static struct resource da9061_regulators_resources[] = {
> +	DEFINE_RES_NAMED(DA9061_IRQ_LDO_LIM, 1, "LDO_LIM", IORESOURCE_IRQ),
> +};
> +
> +static struct resource da9061_thermal_resources[] = {
> +	DEFINE_RES_NAMED(DA9061_IRQ_TEMP, 1, "THERMAL", IORESOURCE_IRQ),
> +};
> +
> +static struct resource da9061_wdt_resources[] = {
> +	DEFINE_RES_NAMED(DA9061_IRQ_WDG_WARN, 1, "WD_WARN", IORESOURCE_IRQ),
> +};
> +
> +static struct resource da9061_onkey_resources[] = {
> +	DEFINE_RES_NAMED(DA9061_IRQ_ONKEY, 1, "ONKEY", IORESOURCE_IRQ),
> +};

These should al be: DEFINE_RES_IO_NAMED()

[...]

>  };
> @@ -142,7 +257,7 @@ static const struct mfd_cell da9062_devs[] = {
>  		.name		= "da9062-watchdog",
>  		.num_resources	= ARRAY_SIZE(da9062_wdt_resources),
>  		.resources	= da9062_wdt_resources,
> -		.of_compatible  = "dlg,da9062-wdt",
> +		.of_compatible  = "dlg,da9062-watchdog",

This should be a separate change.

>  	},
>  	{
>  		.name		= "da9062-thermal",
> @@ -200,7 +315,8 @@ static int da9062_clear_fault_log(struct da9062 *chip)
>  
>  static int da9062_get_device_type(struct da9062 *chip)
>  {
> -	int device_id, variant_id, variant_mrc;
> +	int device_id, variant_id, variant_mrc, variant_vrc;
> +	char *type;
>  	int ret;
>  
>  	ret = regmap_read(chip->regmap, DA9062AA_DEVICE_ID, &device_id);
> @@ -208,6 +324,7 @@ static int da9062_get_device_type(struct da9062 *chip)
>  		dev_err(chip->dev, "Cannot read chip ID.\n");
>  		return -EIO;
>  	}
> +

Sneaky!

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

^ permalink raw reply

* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-11 10:09 UTC (permalink / raw)
  To: Arnd Bergmann, Gabriele Paoloni
  Cc: linux-arm-kernel@lists.infradead.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org, lorenzo.pieralisi@arm.com,
	minyard@acm.org, linux-pci@vger.kernel.org,
	benh@kernel.crashing.org, John Garry, will.deacon@arm.com,
	linux-kernel@vger.kernel.org, xuwei (O), Linuxarm,
	zourongrong@gmail.com, robh+dt@kernel.org, kantyzc@163.com,
	"linux-serial@vger.kernel.org" <linux-seria>
In-Reply-To: <10334260.ztLXZ2Oynd@wuerfel>

Hi, Arnd,

On 2016/11/11 0:07, Arnd Bergmann wrote:
> On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni wrote:
>>
>> Where should we get the range from? For LPC we know that it is going
>> Work on anything that is not used by PCI I/O space, and this is 
>> why we use [0, PCIBIOS_MIN_IO]
> 
> It should be allocated the same way we allocate PCI config space
> segments. This is currently done with the io_range list in
> drivers/pci/pci.c, which isn't perfect but could be extended
> if necessary. Based on what others commented here, I'd rather
> make the differences between ISA/LPC and PCI I/O ranges smaller
> than larger.
> 
>>> Your current version has
>>>
>>>         if (arm64_extio_ops->pfout)                             \
>>>                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
>>>                        addr, value, sizeof(type));             \
>>>
>>> Instead, just subtract the start of the range from the logical
>>> port number to transform it back into a bus-local port number:
>>
>> These accessors do not operate on IO tokens:
>>
>> If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
>> addr is not going to be an I/O token; in fact patch 2/3 imposes that
>> the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to PCIBIOS_MIN_IO
>> we have free physical addresses that the accessors can operate on.
> 
> Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer to
> the logical I/O tokens, the purpose of that macro is really meant
> for allocating PCI I/O port numbers within the address space of
> one bus.
> 
> Note that it's equally likely that whichever next platform needs
> non-mapped I/O access like this actually needs them for PCI I/O space,
> and that will use it on addresses registered to a PCI host bridge.
> 
> If we separate the two steps:
> 
> a) assign a range of logical I/O port numbers to a bus
> b) register a set of helpers for redirecting logical I/O
>    port to a helper function
>
It seems that we need to add a new bus and the corresponding resource management
which can also cover current PCI pio mapping, is it right?

Thanks,
Zhichang

> then I think the code will get cleaner and more flexible.
> It should actually then be able to replace the powerpc
> specific implementation.
> 
> 	Arnd
> 
> .
> 

^ permalink raw reply

* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-11 10:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Mark Rutland
  Cc: catalin.marinas, will.deacon, robh+dt, bhelgaas, olof, arnd,
	linux-arm-kernel, lorenzo.pieralisi, linux-kernel, linuxarm,
	devicetree, linux-pci, linux-serial, minyard, liviu.dudau,
	zourongrong, john.garry, gabriele.paoloni, zhichang.yuan02,
	kantyzc, xuwei5, marc.zyngier
In-Reply-To: <1478806353.7430.137.camel@kernel.crashing.org>

Hi, Ben, Mark,

Thanks for your comments! These are helpful!


On 2016/11/11 3:32, Benjamin Herrenschmidt wrote:
> On Thu, 2016-11-10 at 11:22 +0000, Mark Rutland wrote:
>> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
>>> firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
>>
>> Just to check, do you hook that in your inb/outb/etc?
> 
> Yes.
> 
>> Generally, it would seem nicer if we could have higher-level
>> isa_{inb,outb,whatever} accessors that we could hook separately from
>> other IO.
> 
> Maybe but generally speaking, we don't discriminate accessors per bus,
> ie, readl etc... work on all memory mapped busses, inb... works on all
> busses with an "IO space", at least that's been the idea. It probably
> all comes from the fact that PCI IO and ISA are the same space on
> x86 and most other platforms (not all).
> 
>> We don't necessarily have to move all ISA drivers over to that if we had
>> a separate symbol for that interface.
> 
> What I do on ppc today is that I have a chunk of virtual address space
> that is reserved for "IO space". The first 64k are "reserved" in that
> they route to "the primary" ISA bus (for legacy crap that uses hard
> coded addresses, though I use that for my LPC bus too). I "allocate"
> space for the PCI IO spaces higher in that space. Was I to support more
> LPC busses I could allocate them up there too.
>

I have similar idea as your PPC MMIO.

We notice the prototype of {in/out()} is something like that:

static inline u8 inb(unsigned long addr)
static inline void outb(u8 value, unsigned long addr)

The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
So, could you divide this 'addr' into several bit segments? The top 8 bits is
defined as bus index. For normal direct IO, the bus index is 0. For those bus
device which need indirectIO or some special I/O accessors, when these devices
are initializing, can request to allocate an unique ID to them, and register
their own accessors to the entry which is corresponding to the ID.

In this way, we can support multiple domains, I think.
But I am not sure whether it is feasible, for example, are there some
architectures/platforms had populated the top 8 bits? Do we need to request IO
region from ioport_resource for those devices?  etc...


Thanks,
Zhichang


> The IO resource of a given device thus becomes the actual IO port plus
> the offset of the base of the segment it's in.
> 
> For memory mapped IO, inb/outb will just add the virtual address of
> the base of all IO space to that. The hooking mechanism will pickup
> the stuff that isn't memory mapped.
> 
> It's a bit messy but then IO space performance has never been a huge
> worry since IO cycles tend to be very slow to begin with.
> 
> Note: We also have the ISA memory and ISA FW spaces that we don't have
> good accessors for. They somewhat exist (I think the fbdev layer uses
> some for vga) but it's messy.
> 
> Cheers,
> Ben.
> 
> 
> .
> 

^ permalink raw reply

* Re: [PATCH V3 8/9] thermal: da9062/61: Thermal junction temperature monitoring driver
From: Lukasz Luba @ 2016-11-11  9:57 UTC (permalink / raw)
  To: Steve Twiss, Eduardo Valentin, LINUX-KERNEL, LINUX-PM, Zhang Rui
  Cc: DEVICETREE, Dmitry Torokhov, Guenter Roeck, LINUX-INPUT,
	LINUX-WATCHDOG, Lee Jones, Liam Girdwood, Mark Brown,
	Mark Rutland, Rob Herring, Support Opensource, Wim Van Sebroeck
In-Reply-To: <6ED8E3B22081A4459DAC7699F3695FB7018CCE91AE@SW-EX-MBX02.diasemi.com>

Hi Steve,

On 09/11/16 18:20, Steve Twiss wrote:
> On 02 November 2016 13:29, Lukasz Luba wrote:
> [...]
>
>> Apart from these 2 comments, 10sec is not to long
>> (waiting for the temperature change)?
>
> Hi Lukasz,
>
> Are you saying the maximum polling time is too long or too short if it
> is fixed in the driver at 10 seconds?
In my opinion 10s is too long.
>
> Certainly 10 seconds can be seen as either too long or too short a time
> when waiting for the temperature to fall-back below a threshold.
> But, this maximum polling time will be application dependent I think.
>
> However, this is a repeated polling event notifying of a warning
> over-temperature condition, so, it is already known that the
> temperature is above the threshold and action should already be
> in progress to reduce the temperature.
In this case we have precise start time when we should e.g. throttle
the CPU, because this interrupt will be fired by the hardware just
after the real temperature change. We do not have precise end time
for the throttling process, though. The function .get_temp
may return stale data which was read out some time ago
(< 3s or max 10s).
The hole system performance may suffer for too long (because the
temperature could drop to i.e. 100degC).

On the other hand, when we consider that this is just a binary flag
reacting on 125degC threshold then maybe there is a point of cooling
down the PMIC for longer time.
You are right it is application and system specific (i.e. how many
other temperature sensors is registered in the system and what
decisions we can make based on them i.e. in IPA).
>
> #define DA9062_DEFAULT_POLLING_MS_PERIOD	3000
> #define DA9062_MAX_POLLING_MS_PERIOD		10000
> #define DA9062_MIN_POLLING_MS_PERIOD		1000
>
> The TEMP_WARN first level temperature supervision is intended for
> non-invasive temperature controlling measures for cooling the system
> and are left to the host software. This first level temperature
> TEMP_WARN (125 degC) is only +15degC off the next TEMP_CRIT
> (140 degC) temperature threshold. And this TEMP_CRIT is where
> the hardware will automatically shutdown.
>
> I suppose it all depends on how fast the temperature is expected to
> rise and fall.
>
> In any case, this 10 second polling maximum value was provided as part
> of guidance from a specific solution with this hardware. It would be expected
> that any final implementation will also include a notify() function and any
> of these settings could be altered to match the application where
> appropriate.
>
> I've added a comment above these defined variables for the next code
> patch.
Fair enough. You can mention about the throttling side effect so that
engineers working on bring-up will be aware of this particular knob
during the experiments.

Best Regards,
Lukasz


>
>> On 31/10/16 16:02, Steve Twiss wrote:
>>> From: Steve Twiss <stwiss.opensource@diasemi.com>
>>>
>>> +static int da9062_thermal_probe(struct platform_device *pdev)
>>> +{
>>> +	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
>>> +	struct da9062_thermal *thermal;
>>> +	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
>>> +	const struct of_device_id *match;
>>> +	int ret = 0;
>>> +
>>> +	match = of_match_node(da9062_compatible_reg_id_table,
>>> +			      pdev->dev.of_node);
>>> +	if (!match)
>>> +		return -ENXIO;
>>> +
>>> +	if (pdev->dev.of_node) {
>>> +		if (!of_property_read_u32(pdev->dev.of_node,
>>> +					"dlg,tjunc-temp-polling-period-ms",
>>> +					&pp_tmp)) {
>>> +			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
>>> +				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
>>> +				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
>>
>> Maybe it's worth to add some print here just to mention about
>> the DT value out of range. When you saw a dmesg with
>> this print on some bug report, you would know about wrong DT entry
>> (even if debug was not set).
>
> I can add a dev_warn() here explaining the invalid configuration.
>
> [...]
>
>>> +static int da9062_thermal_remove(struct platform_device *pdev)
>>> +{
>>> +	struct	da9062_thermal *thermal = platform_get_drvdata(pdev);
>>> +
>>> +	free_irq(thermal->irq, thermal);
>>> +	thermal_zone_device_unregister(thermal->zone);
>>> +	cancel_delayed_work_sync(&thermal->work);
>>
>> You should change the order for these two functions
>> and cancel the work before unregistering thermal zone device.
>
> ok
>
> Regards,
> Steve
>

^ permalink raw reply

* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Horng-Shyang Liao @ 2016-11-11  9:34 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu,
	Jo
In-Reply-To: <CABb+yY3mi3M0xqNao=DRYN6Np0LOBfS669iUa_hWo2w1MmM1sw@mail.gmail.com>

On Fri, 2016-11-11 at 11:15 +0530, Jassi Brar wrote:
> On Thu, Nov 10, 2016 at 4:45 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
> >> Hi,
> >>
> >> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
> >> to help write registers with critical time limitation, such as
> >> updating display configuration during the vblank. It controls Global
> >> Command Engine (GCE) hardware to achieve this requirement.
> >>
> >> These patches have a build dependency on top of v4.9-rc1.
> >>
> >> Changes since v15:
> >>  - separate "suspend and resume" patch from "save energy" patch
> >>  - don't stop running tasks in cmdq_suspend()
> >>    (i.e. leave no running tasks guarantee to clients)
> >>
> >> Best regards,
> >> HS Liao
> >>
> >> HS Liao (5):
> >>   dt-bindings: soc: Add documentation for the MediaTek GCE unit
> >>   CMDQ: Mediatek CMDQ driver
> >>   arm64: dts: mt8173: Add GCE node
> >>   CMDQ: suspend and resume
> >>   CMDQ: save energy
> >>
> >>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  43 ++
> >>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
> >>  drivers/mailbox/Kconfig                            |  10 +
> >>  drivers/mailbox/Makefile                           |   2 +
> >>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 632 +++++++++++++++++++++
> >>  drivers/soc/mediatek/Kconfig                       |  11 +
> >>  drivers/soc/mediatek/Makefile                      |   1 +
> >>  drivers/soc/mediatek/mtk-cmdq-helper.c             | 310 ++++++++++
> >>  include/linux/mailbox/mtk-cmdq-mailbox.h           |  67 +++
> >>  include/linux/soc/mediatek/mtk-cmdq.h              | 182 ++++++
> >>  10 files changed, 1268 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> >>  create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
> >>  create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
> >>  create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
> >>  create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
> >>
> >
> >
> > Hi Jassi, Matthias,
> >
> > Sorry to disturb you.
> >
> No, you don't disturb, but the controller driver and protocol driver,
> introduced in the same patch, does :)   So does the suspend/resume
> support (patch 4&5) added  separately as a patch on top. Please
> reorganise the patchset.
> 
> Thanks.

Hi Jassi,

Do you mean
1. split controller driver and protocol driver as two patches,
2. merge patch 4&5 into one patch, and
3. reorganize the patchset as "(1) binding doc (2) controller driver
   (3) protocol driver (4) devicetree (5) energy patch" ?

Thanks,
HS

^ permalink raw reply

* [PATCH 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Shawn Lin @ 2016-11-11  9:16 UTC (permalink / raw)
  To: Rob Herring, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
	Shawn Lin
In-Reply-To: <1478855766-151673-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Add rockchip serial flash controller driver

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

---

 MAINTAINERS                        |   8 +
 drivers/mtd/spi-nor/Kconfig        |   7 +
 drivers/mtd/spi-nor/Makefile       |   1 +
 drivers/mtd/spi-nor/rockchip-sfc.c | 953 +++++++++++++++++++++++++++++++++++++
 4 files changed, 969 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7..eb7e06d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10266,6 +10266,14 @@ L:	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 S:	Odd Fixes
 F:	drivers/tty/serial/rp2.*
 
+ROCKCHIP SERIAL FLASH CONTROLLER DRIVER
+M:	Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+L:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
+L:	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
+F:	drivers/mtd/spi-nor/rockchip-sfc.c
+
 ROSE NETWORK LAYER
 M:	Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
 L:	linux-hams-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 4a682ee..48c5e0e 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -65,6 +65,13 @@ config SPI_HISI_SFC
 	help
 	  This enables support for hisilicon SPI-NOR flash controller.
 
+config SPI_ROCKCHIP_SFC
+	tristate "Rockchip Serial Flash Controller(SFC)"
+	depends on ARCH_ROCKCHIP || COMPILE_TEST
+	depends on HAS_IOMEM && HAS_DMA
+	help
+	  This enables support for rockchip serial flash controller.
+
 config SPI_NXP_SPIFI
 	tristate "NXP SPI Flash Interface (SPIFI)"
 	depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 121695e..364d4c6 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
 obj-$(CONFIG_SPI_HISI_SFC)	+= hisi-sfc.o
 obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
+obj-$(CONFIG_SPI_ROCKCHIP_SFC)	+= rockchip-sfc.o
diff --git a/drivers/mtd/spi-nor/rockchip-sfc.c b/drivers/mtd/spi-nor/rockchip-sfc.c
new file mode 100644
index 0000000..83930d6
--- /dev/null
+++ b/drivers/mtd/spi-nor/rockchip-sfc.c
@@ -0,0 +1,953 @@
+/*
+ * Rockchip Serial Flash Controller Driver
+ *
+ * Copyright (c) 2016, Rockchip Inc.
+ * Author: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * 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.
+ *
+ * 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/bitops.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* System control */
+#define SFC_CTRL			0x0
+#define  SFC_CTRL_COMMON_BITS_1		0x0
+#define  SFC_CTRL_COMMON_BITS_2		0x1
+#define  SFC_CTRL_COMMON_BITS_4		0x2
+#define  SFC_CTRL_DATA_BITS_SHIFT	12
+#define  SFC_CTRL_DATA_BITS_MASK	0x3
+#define  SFC_CTRL_ADDR_BITS_SHIFT	10
+#define  SFC_CTRL_ADDR_BITS_MASK	0x3
+#define  SFC_CTRL_CMD_BITS_SHIFT	8
+#define  SFC_CTRL_CMD_BITS_MASK		0x3
+#define  SFC_CTRL_IDLE_CYC_SHIFT	4
+#define  SFC_CTRL_IDLE_CYC_MASK		0xf
+#define  SFC_CTRL_PHASE_SEL_SHIFT	1
+#define  SFC_CTRL_PHASE_SEL_NEGETIVE	0x1
+#define  SFC_CTRL_PHASE_SEL_POSITIVE	0x0
+#define  SFC_CTRL_MODE_SEL_SHIFT	0
+#define  SFC_CTRL_MODE_MASK		0x1
+
+/* Interrypt mask */
+#define SFC_IMR				0x4
+#define  SFC_IMR_RX_FULL		BIT(0)
+#define  SFC_IMR_RX_UFLOW		BIT(1)
+#define  SFC_IMR_TX_OFLOW		BIT(2)
+#define  SFC_IMR_TX_EMPTY		BIT(3)
+#define  SFC_IMR_TRAN_FINISH		BIT(4)
+#define  SFC_IMR_BUS_ERR		BIT(5)
+#define  SFC_IMR_NSPI_ERR		BIT(6)
+#define  SFC_IMR_DMA			BIT(7)
+/* Interrupt clear */
+#define SFC_ICLR			0x8
+#define  SFC_ICLR_RX_FULL		BIT(0)
+#define  SFC_ICLR_RX_UFLOW		BIT(1)
+#define  SFC_ICLR_TX_OFLOW		BIT(2)
+#define  SFC_ICLR_TX_EMPTY		BIT(3)
+#define  SFC_ICLR_TRAN_FINISH		BIT(4)
+#define  SFC_ICLR_BUS_ERR		BIT(5)
+#define  SFC_ICLR_NSPI_ERR		BIT(6)
+#define  SFC_ICLR_DMA			BIT(7)
+/* FIFO threshold level */
+#define SFC_FTLR			0xc
+#define  SFC_FTLR_TX_SHIFT		0
+#define  SFC_FTLR_TX_MASK		0x1f
+#define  SFC_FTLR_RX_SHIFT		8
+#define  SFC_FTLR_RX_MASK		0x1f
+/* Reset FSM and FIFO */
+#define SFC_RCVR			0x10
+#define  SFC_RCVR_RESET			BIT(0)
+/* Enhanced mode */
+#define SFC_AX				0x14
+#define  SFC_AX_MODE_BIT_SHIFT		0
+#define  SFC_AX_MODE_BIT_MASK		0xff
+/* Address Bit number */
+#define SFC_ABIT			0x18
+#define  SFC_ABIT_ADD_SHIFT		0
+#define  SFC_ABIT_ADD_MASK		0x1f
+/* Interrupt status */
+#define SFC_ISR				0x1c
+#define  SFC_ISR_COMMON_ACTIVE		0x1
+#define  SFC_ISR_COMMON_MASK		0x1
+#define  SFC_ISR_RX_FULL_SHIFT		0
+#define  SFC_ISR_RX_UFLOW_SHIFT		1
+#define  SFC_ISR_TX_OFLOW_SHIFT		2
+#define  SFC_ISR_TX_EMPTY_SHIFT		3
+#define  SFC_ISR_TX_FINISH_SHIFT	4
+#define  SFC_ISR_BUS_ERR_SHIFT		5
+#define  SFC_ISR_NSPI_ERR_SHIFT		6
+#define  SFC_ISR_DMA_SHIFT		7
+/* FIFO status */
+#define SFC_FSR				0x20
+#define  SFC_FSR_TX_IS_FULL		0x1
+#define  SFC_FSR_TX_FULL_SHIFT		0
+#define  SFC_FSR_TX_FULL_MASK		0x1
+#define  SFC_FSR_TX_IS_EMPTY		0x1
+#define  SFC_FSR_TX_EMPTY_SHIFT		1
+#define  SFC_FSR_TX_EMPTY_MASK		0x1
+#define  SFC_FSR_RX_IS_EMPTY		0x1
+#define  SFC_FSR_RX_EMPTY_SHIFT		2
+#define  SFC_FSR_RX_EMPTY_MASK		0x1
+#define  SFC_FSR_RX_IS_FULL		0x1
+#define  SFC_FSR_RX_FULL_SHIFT		3
+#define  SFC_FSR_RX_FULL_MASK		0x1
+#define  SFC_FSR_TX_WATER_LVL_SHIFT	8
+#define  SFC_FSR_TX_WATER_LVL_MASK	0x1f
+#define  SFC_FSR_RX_WATER_LVL_SHIFT	16
+#define  SFC_FSR_RX_WATER_LVL_MASK	0x1f
+/* FSM status */
+#define SFC_SR				0x24
+#define  SFC_SR_IS_IDLE			0x0
+#define  SFC_SR_IS_BUSY			0x1
+/* Raw interrupt status */
+#define SFC_RISR			0x28
+#define  SFC_RISR_COMMON_ACTIVE		0x1
+#define  SFC_RISR_COMMON_MASK		0x1
+#define  SFC_RISR_RX_FULL_SHIFT		0
+#define  SFC_RISR_RX_UFLOW_SHIFT	1
+#define  SFC_RISR_TX_OFLOW_SHIFT	2
+#define  SFC_RISR_TX_EMPTY_SHIFT	3
+#define  SFC_RISR_TRAN_FINISH_SHIFT	4
+#define  SFC_RISR_BUS_ERR_SHIFT		5
+#define  SFC_RISR_NSPI_ERR_SHIFT	6
+#define  SFC_RISR_DMA_SHIFT		7
+/* Master trigger */
+#define SFC_DMA_TRIGGER			0x80
+/* Src or Dst addr for master */
+#define SFC_DMA_ADDR			0x84
+/* Command */
+#define SFC_CMD				0x100
+#define  SFC_CMD_IDX_SHIFT		0
+#define  SFC_CMD_IDX_MASK		0xff
+#define  SFC_CMD_DUMMY_SHIFT		8
+#define  SFC_CMD_DUMMY_MASK		0xf
+#define  SFC_CMD_DIR_RD			0
+#define  SFC_CMD_DIR_WR			1
+#define  SFC_CMD_DIR_SHIFT		12
+#define  SFC_CMD_DIR_MASK		0x1
+#define  SFC_CMD_CONTI_RD		1
+#define  SFC_CMD_CONTI_RD_SHIFT		13
+#define  SFC_CMD_ADDR_ZERO		0
+#define  SFC_CMD_ADDR_24BITS		0x1
+#define  SFC_CMD_ADDR_32BITS		0x2
+#define  SFC_CMD_ADDR_FRS		0x3
+#define  SFC_CMD_ADDR_SHIFT		14
+#define  SFC_CMD_TRAN_BYTES_SHIFT	16
+#define  SFC_CMD_TRAN_BYTES_MASK	0x3fff
+#define  SFC_CMD_CS_SHIFT		30
+#define  SFC_CMD_CS_MASK		0x3
+/* Address */
+#define SFC_ADDR			0x104
+/* Data */
+#define SFC_DATA			0x108
+
+#define SFC_MAX_CHIP_NUM		4
+#define SFC_MAX_IDLE_RETRY		10000
+#define SFC_WAIT_IDLE_TIMEOUT		1000000
+#define SFC_DMA_MAX_LEN			0x4000
+#define SFC_DMA_MAX_MASK		(SFC_DMA_MAX_LEN - 1)
+#define SFC_IRQ_TRAN_FINISH		BIT(SFC_RISR_TRAN_FINISH_SHIFT)
+
+enum rockchip_sfc_iftype {
+	IF_TYPE_STD,
+	IF_TYPE_DUAL,
+	IF_TYPE_QUAD,
+};
+
+struct rockchip_sfc {
+	struct device *dev;
+	struct mutex lock;
+	void __iomem *regbase;
+	struct clk *hclk;
+	struct clk *clk;
+	void *buffer;
+	dma_addr_t dma_buffer;
+	struct completion cp;
+	struct spi_nor	*nor[SFC_MAX_CHIP_NUM];
+	u32 num_chip;
+	bool use_dma;
+	bool negative_edge;
+};
+
+struct rockchip_sfc_priv {
+	u32 cs;
+	u32 clk_rate;
+	struct rockchip_sfc *sfc;
+};
+
+static int get_if_type(enum read_mode flash_read)
+{
+	enum rockchip_sfc_iftype if_type;
+
+	switch (flash_read) {
+	case SPI_NOR_DUAL:
+		if_type = IF_TYPE_DUAL;
+		break;
+	case SPI_NOR_QUAD:
+		if_type = IF_TYPE_QUAD;
+		break;
+	case SPI_NOR_NORMAL:
+	case SPI_NOR_FAST:
+	default:
+		if_type = IF_TYPE_STD;
+		break;
+	}
+
+	return if_type;
+}
+
+static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
+{
+	unsigned long timeout = jiffies + HZ;
+	int err = -ETIMEDOUT;
+	u32 status;
+
+	writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
+
+	while (time_before(jiffies, timeout)) {
+		status = readl_relaxed(sfc->regbase + SFC_RCVR);
+		if (!(status & SFC_RCVR_RESET)) {
+			err = 0;
+			break;
+		}
+		msleep(20);
+	}
+
+	if (err)
+		dev_err(sfc->dev, "SFC reset never finished\n");
+
+	writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
+		       SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
+		       SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
+		       SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
+		       sfc->regbase + SFC_ICLR);
+	return err;
+}
+
+static int rockchip_sfc_init(struct rockchip_sfc *sfc)
+{
+	int err;
+
+	err = rockchip_sfc_reset(sfc);
+	if (err)
+		return err;
+
+	/* Mask all eight interrupts */
+	writel_relaxed(0xff, sfc->regbase + SFC_IMR);
+	/* Phase configure */
+	if (sfc->negative_edge)
+		writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE <<
+			       SFC_CTRL_PHASE_SEL_SHIFT,
+			       sfc->regbase + SFC_CTRL);
+	return 0;
+}
+
+static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	int ret;
+
+	mutex_lock(&sfc->lock);
+
+	ret = clk_set_rate(sfc->clk, priv->clk_rate);
+	if (ret)
+		goto out;
+
+	ret = clk_prepare_enable(sfc->clk);
+	if (ret)
+		goto out;
+
+	return 0;
+
+out:
+	mutex_unlock(&sfc->lock);
+	return ret;
+}
+
+static void rockchip_sfc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+
+	clk_disable_unprepare(sfc->clk);
+	mutex_unlock(&sfc->lock);
+}
+
+static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc)
+{
+	unsigned long timeout = jiffies + 2 * HZ;
+	int err = -ETIMEDOUT;
+	u32 status;
+
+	/*
+	 * Note: tx and rx share the same fifo, so the rx's water level
+	 * is the same as rx's, which means this function could be reused
+	 * for checking the read operations as well.
+	 */
+	while (time_before(jiffies, timeout)) {
+		status = readl_relaxed(sfc->regbase + SFC_FSR);
+		if (((status >> SFC_FSR_TX_EMPTY_SHIFT) &
+		     SFC_FSR_TX_EMPTY_MASK) == SFC_FSR_TX_IS_EMPTY) {
+			err = 0;
+			break;
+		}
+		msleep(20);
+	}
+
+	if (err)
+		dev_err(sfc->dev, "SFC tx never empty\n");
+
+	return err;
+}
+
+static int rockchip_sfc_op_reg(struct spi_nor *nor,
+				u8 opcode, int len, u8 optype)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	u32 reg;
+
+	if (((readl_relaxed(sfc->regbase + SFC_FSR) >> SFC_FSR_TX_EMPTY_SHIFT) &
+	      SFC_FSR_TX_EMPTY_MASK) != SFC_FSR_TX_IS_EMPTY ||
+	     ((readl_relaxed(sfc->regbase + SFC_FSR) >>
+	       SFC_FSR_RX_EMPTY_SHIFT) &
+	      SFC_FSR_RX_EMPTY_MASK) != SFC_FSR_RX_IS_EMPTY ||
+	     (readl_relaxed(sfc->regbase + SFC_SR) == SFC_SR_IS_BUSY))
+		rockchip_sfc_reset(sfc);
+
+	reg = (opcode & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT;
+	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
+	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
+	reg |= (optype & SFC_CMD_DIR_MASK) << SFC_CMD_DIR_SHIFT;
+
+	writel_relaxed(reg, sfc->regbase + SFC_CMD);
+
+	return rockchip_sfc_wait_op_finish(sfc);
+}
+
+static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode,
+				 u8 *buf, int len)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	int ret;
+	u32 tmp;
+	u32 i;
+
+	ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD);
+	if (ret)
+		return ret;
+
+	while (len > 0) {
+		tmp = readl_relaxed(sfc->regbase + SFC_DATA);
+		for (i = 0; i < len; i++)
+			*buf++ = (u8)((tmp >> (i * 8)) & 0xff);
+		len = len - 4;
+	}
+
+	return 0;
+}
+
+static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode,
+				  u8 *buf, int len)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	u32 words, i;
+
+	/* Align bytes to words */
+	words = (len + 3) >> 2;
+
+	for (i = 0; i < words; i++)
+		writel_relaxed(*(buf + 4 * i), sfc->regbase + SFC_DATA);
+
+	return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR);
+}
+
+static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to,
+				     dma_addr_t dma_buf, size_t len, u8 op_type)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	u32 reg;
+	u8 if_type = 0;
+
+	init_completion(&sfc->cp);
+
+	writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
+		       SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
+		       SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
+		       SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
+		       sfc->regbase + SFC_ICLR);
+
+	/* Enable transfer complete interrupt */
+	reg = readl_relaxed(sfc->regbase + SFC_IMR);
+	reg &= ~SFC_IMR_TRAN_FINISH;
+	writel_relaxed(reg, sfc->regbase + SFC_IMR);
+
+	if (op_type == SFC_CMD_DIR_WR)
+		reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
+		      ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
+		       SFC_CMD_IDX_SHIFT);
+	else
+		reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
+		      ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
+		       SFC_CMD_IDX_SHIFT);
+
+	reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
+		: SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;
+
+	if_type = get_if_type(nor->flash_read);
+	writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
+		       if_type << SFC_CTRL_ADDR_BITS_SHIFT |
+		       if_type << SFC_CTRL_CMD_BITS_SHIFT |
+		       sfc->negative_edge ?
+		       SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
+		       SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
+		       sfc->regbase + SFC_CTRL);
+
+	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
+	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
+
+	if (op_type == SFC_CMD_DIR_RD)
+		reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
+			SFC_CMD_DUMMY_SHIFT;
+
+	/* Should minus one as 0x0 means 1 bit flash address */
+	writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
+	writel_relaxed(reg, sfc->regbase + SFC_CMD);
+	writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
+	writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR);
+
+	/* Start dma */
+	writel_relaxed(0x1, sfc->regbase + SFC_DMA_TRIGGER);
+
+	/* Wait for the interrupt. */
+	if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
+		dev_err(sfc->dev, "DMA wait for transfer finish timeout.");
+		return -ETIMEDOUT;
+	}
+
+	/* Disable transfer finish interrupt */
+	reg = readl_relaxed(sfc->regbase + SFC_IMR);
+	reg |= SFC_IMR_TRAN_FINISH;
+	writel_relaxed(reg, sfc->regbase + SFC_IMR);
+
+	return rockchip_sfc_wait_op_finish(sfc);
+}
+
+static inline int rockchip_sfc_pio_write(struct rockchip_sfc *sfc, u_char *buf,
+					 size_t len)
+{
+	u32 words, tx_wl, count, i;
+	unsigned long timeout;
+	int ret = 0;
+	u32 *tbuf = (u32 *)buf;
+
+	/* Align bytes to words */
+	words = (len + 3) >> 2;
+
+	while (words) {
+		tx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
+			 SFC_FSR_TX_WATER_LVL_SHIFT) &
+			 SFC_FSR_TX_WATER_LVL_MASK;
+
+		if (tx_wl > 0) {
+			count = min_t(u32, words, tx_wl);
+			for (i = 0; i < count; i++) {
+				writel_relaxed(*tbuf++,
+					       sfc->regbase + SFC_DATA);
+				words--;
+			}
+
+			if (words == 0)
+				break;
+			timeout = 0;
+		} else {
+			mdelay(1);
+			if (timeout++ > SFC_MAX_IDLE_RETRY) {
+				ret = -ETIMEDOUT;
+				break;
+			}
+		}
+	}
+
+	if (ret)
+		return ret;
+	else
+		return rockchip_sfc_wait_op_finish(sfc);
+}
+
+static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
+					size_t len)
+{
+	u32 words, rx_wl, count, i;
+	unsigned long timeout;
+	int ret = 0;
+	u32 tmp;
+	u32 *tbuf = (u32 *)buf;
+	u_char *tbuf2;
+
+	words = len >> 2;
+	/* Get the remained bytes */
+	len = len & 0x3;
+
+	while (words) {
+		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
+			 SFC_FSR_RX_WATER_LVL_SHIFT) &
+			 SFC_FSR_RX_WATER_LVL_MASK;
+
+		if (rx_wl > 0) {
+			count = min_t(u32, words, rx_wl);
+			for (i = 0; i < count; i++) {
+				*tbuf++ = readl_relaxed(sfc->regbase +
+							SFC_DATA);
+				words--;
+			}
+
+			if (words == 0)
+				break;
+			timeout = 0;
+		} else {
+			mdelay(1);
+			if (timeout++ > SFC_MAX_IDLE_RETRY) {
+				ret = -ETIMEDOUT;
+				break;
+			}
+		}
+	}
+
+	if (ret)
+		return ret;
+
+	/* Read the remained bytes */
+	timeout = 0;
+	tbuf2 = (u_char *)tbuf;
+	while (len) {
+		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
+			 SFC_FSR_RX_WATER_LVL_SHIFT) &
+			 SFC_FSR_RX_WATER_LVL_MASK;
+		if (rx_wl > 0) {
+			tmp = readl_relaxed(sfc->regbase + SFC_DATA);
+			for (i = 0; i < len; i++)
+				tbuf2[i] = (u8)((tmp >> (i * 8)) & 0xff);
+			goto done;
+		} else {
+			mdelay(1);
+			if (timeout++ > SFC_MAX_IDLE_RETRY) {
+				ret = -ETIMEDOUT;
+				break;
+			}
+		}
+	}
+done:
+	if (ret)
+		return ret;
+	else
+		return rockchip_sfc_wait_op_finish(sfc);
+}
+
+static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to,
+				     size_t len, u_char *buf, u8 op_type)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	u32 reg;
+	u8 if_type = 0;
+
+	if (op_type == SFC_CMD_DIR_WR)
+		reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
+		      ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
+		       SFC_CMD_IDX_SHIFT);
+	else
+		reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
+		      ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
+		       SFC_CMD_IDX_SHIFT);
+
+	reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
+		: SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;
+
+	if_type = get_if_type(nor->flash_read);
+	writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
+		       if_type << SFC_CTRL_ADDR_BITS_SHIFT |
+		       if_type << SFC_CTRL_CMD_BITS_SHIFT |
+		       sfc->negative_edge ?
+		       SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
+		       SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
+		       sfc->regbase + SFC_CTRL);
+
+	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
+	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
+
+	if (op_type == SFC_CMD_DIR_RD)
+		reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
+			SFC_CMD_DUMMY_SHIFT;
+
+	/* Should minus one as 0x0 means 1 bit flash address */
+	writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
+	writel_relaxed(reg, sfc->regbase + SFC_CMD);
+	writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
+
+	if (op_type == SFC_CMD_DIR_WR)
+		return rockchip_sfc_pio_write(sfc, buf, len);
+	else
+		return rockchip_sfc_pio_read(sfc, buf, len);
+}
+
+static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from, size_t len,
+				 u_char *read_buf)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	size_t offset;
+	int ret;
+	dma_addr_t dma_addr = 0;
+
+	if (!sfc->use_dma)
+		goto no_dma;
+
+	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
+		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
+
+		dma_addr = dma_map_single(NULL, (void *)read_buf,
+					  trans, DMA_FROM_DEVICE);
+		if (dma_mapping_error(sfc->dev, dma_addr))
+			dma_addr = 0;
+
+		/* Fail to map dma, use pre-allocated area instead */
+		ret = rockchip_sfc_dma_transfer(nor, from + offset,
+						dma_addr ? dma_addr :
+						sfc->dma_buffer,
+						trans, SFC_CMD_DIR_RD);
+		if (ret) {
+			dev_warn(nor->dev, "DMA read timeout\n");
+			return ret;
+		}
+		if (!dma_addr)
+			memcpy(read_buf + offset, sfc->buffer, trans);
+	}
+
+	return len;
+
+no_dma:
+	ret = rockchip_sfc_pio_transfer(nor, from, len,
+					read_buf, SFC_CMD_DIR_RD);
+	if (ret) {
+		dev_warn(nor->dev, "PIO read timeout\n");
+		return ret;
+	}
+	return len;
+}
+
+static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
+				  size_t len, const u_char *write_buf)
+{
+	struct rockchip_sfc_priv *priv = nor->priv;
+	struct rockchip_sfc *sfc = priv->sfc;
+	size_t offset;
+	int ret;
+	dma_addr_t dma_addr = 0;
+
+	if (!sfc->use_dma)
+		goto no_dma;
+
+	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
+		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
+
+		dma_addr = dma_map_single(NULL, (void *)write_buf,
+					  trans, DMA_TO_DEVICE);
+		if (dma_mapping_error(sfc->dev, dma_addr)) {
+			dma_addr = 0;
+			memcpy(sfc->buffer, write_buf + offset, trans);
+		}
+
+		/* Fail to map dma, use pre-allocated area instead */
+		ret = rockchip_sfc_dma_transfer(nor, to + offset,
+						dma_addr ? dma_addr :
+						sfc->dma_buffer,
+						trans, SFC_CMD_DIR_WR);
+		if (dma_addr)
+			dma_unmap_single(NULL, dma_addr,
+					 trans, DMA_TO_DEVICE);
+		if (ret) {
+			dev_warn(nor->dev, "DMA write timeout\n");
+			return ret;
+		}
+	}
+
+	return len;
+no_dma:
+	ret = rockchip_sfc_pio_transfer(nor, to, len,
+					(u_char *)write_buf, SFC_CMD_DIR_WR);
+	if (ret) {
+		dev_warn(nor->dev, "PIO write timeout\n");
+		return ret;
+	}
+	return len;
+}
+
+/**
+ * Get spi flash device information and register it as a mtd device.
+ */
+static int rockchip_sfc_register(struct device_node *np,
+				 struct rockchip_sfc *sfc)
+{
+	struct device *dev = sfc->dev;
+	struct spi_nor *nor;
+	struct rockchip_sfc_priv *priv;
+	struct mtd_info *mtd;
+	int ret;
+
+	nor = devm_kzalloc(dev, sizeof(*nor), GFP_KERNEL);
+	if (!nor)
+		return -ENOMEM;
+
+	nor->dev = dev;
+	spi_nor_set_flash_node(nor, np);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(np, "reg", &priv->cs);
+	if (ret) {
+		dev_err(dev, "No reg property for %s\n",
+			np->full_name);
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "spi-max-frequency",
+			&priv->clk_rate);
+	if (ret) {
+		dev_err(dev, "No spi-max-frequency property for %s\n",
+			np->full_name);
+		return ret;
+	}
+
+	priv->sfc = sfc;
+	nor->priv = priv;
+
+	nor->prepare = rockchip_sfc_prep;
+	nor->unprepare = rockchip_sfc_unprep;
+	nor->read_reg = rockchip_sfc_read_reg;
+	nor->write_reg = rockchip_sfc_write_reg;
+	nor->read = rockchip_sfc_read;
+	nor->write = rockchip_sfc_write;
+	nor->erase = NULL;
+	ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+	if (ret)
+		return ret;
+
+	mtd = &nor->mtd;
+	mtd->name = np->name;
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret)
+		return ret;
+
+	sfc->nor[sfc->num_chip] = nor;
+	sfc->num_chip++;
+	return 0;
+}
+
+static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc)
+{
+	int i;
+
+	for (i = 0; i < sfc->num_chip; i++)
+		mtd_device_unregister(&sfc->nor[i]->mtd);
+}
+
+static int rockchip_sfc_register_all(struct rockchip_sfc *sfc)
+{
+	struct device *dev = sfc->dev;
+	struct device_node *np;
+	int ret;
+
+	for_each_available_child_of_node(dev->of_node, np) {
+		ret = rockchip_sfc_register(np, sfc);
+		if (ret)
+			goto fail;
+
+		if (sfc->num_chip == SFC_MAX_CHIP_NUM) {
+			dev_warn(dev, "Exceeds the max cs limitation\n");
+			break;
+		}
+	}
+
+	return 0;
+
+fail:
+	dev_err(dev, "Failed to register all chip\n");
+	rockchip_sfc_unregister_all(sfc);
+	return ret;
+}
+
+static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
+{
+	struct rockchip_sfc *sfc = dev_id;
+	u32 reg;
+
+	reg = readl_relaxed(sfc->regbase + SFC_RISR);
+	dev_dbg(sfc->dev, "Get irq: 0x%x\n", reg);
+
+	/* Clear interrupt */
+	writel_relaxed(reg, sfc->regbase + SFC_ICLR);
+
+	if (reg & SFC_IRQ_TRAN_FINISH)
+		complete(&sfc->cp);
+
+	return IRQ_HANDLED;
+}
+
+static int rockchip_sfc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct rockchip_sfc *sfc;
+	int ret;
+
+	sfc = devm_kzalloc(dev, sizeof(*sfc), GFP_KERNEL);
+	if (!sfc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, sfc);
+	sfc->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sfc->regbase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sfc->regbase))
+		return PTR_ERR(sfc->regbase);
+
+	sfc->clk = devm_clk_get(&pdev->dev, "sfc");
+	if (IS_ERR(sfc->clk)) {
+		dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
+		return PTR_ERR(sfc->clk);
+	}
+
+	sfc->hclk = devm_clk_get(&pdev->dev, "hsfc");
+	if (IS_ERR(sfc->hclk)) {
+		dev_err(&pdev->dev, "Failed to get sfc ahp clk\n");
+		return PTR_ERR(sfc->hclk);
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_warn(dev, "Unable to set dma mask\n");
+		return ret;
+	}
+
+	sfc->buffer = dmam_alloc_coherent(dev, SFC_DMA_MAX_LEN,
+			&sfc->dma_buffer, GFP_KERNEL);
+	if (!sfc->buffer)
+		return -ENOMEM;
+
+	mutex_init(&sfc->lock);
+
+	ret = clk_prepare_enable(sfc->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable hclk\n");
+		goto err_hclk;
+	}
+
+	ret = clk_prepare_enable(sfc->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable clk\n");
+		goto err_clk;
+	}
+
+	if (of_property_read_bool(sfc->dev->of_node, "rockchip,sfc-no-dma"))
+		sfc->use_dma = false;
+	else
+		sfc->use_dma = true;
+
+	if (of_device_is_compatible(sfc->dev->of_node,
+				    "rockchip,rk1108-sfc"))
+		sfc->negative_edge = true;
+	else
+		sfc->negative_edge = false;
+
+	/* Find the irq */
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get the irq\n");
+		goto err_irq;
+	}
+
+	ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
+			       0, pdev->name, sfc);
+	if (ret) {
+		dev_err(dev, "Failed to request irq\n");
+		goto err_irq;
+	}
+
+	ret = rockchip_sfc_init(sfc);
+	if (ret)
+		goto err_init;
+
+	ret = rockchip_sfc_register_all(sfc);
+	if (ret)
+		goto err_init;
+
+	clk_disable_unprepare(sfc->clk);
+	return 0;
+
+err_irq:
+err_init:
+	clk_disable_unprepare(sfc->clk);
+err_clk:
+	clk_disable_unprepare(sfc->hclk);
+err_hclk:
+	mutex_destroy(&sfc->lock);
+	return ret;
+}
+
+static int rockchip_sfc_remove(struct platform_device *pdev)
+{
+	struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
+
+	rockchip_sfc_unregister_all(sfc);
+	mutex_destroy(&sfc->lock);
+	clk_disable_unprepare(sfc->clk);
+	clk_disable_unprepare(sfc->hclk);
+	return 0;
+}
+
+static const struct of_device_id rockchip_sfc_dt_ids[] = {
+	{ .compatible = "rockchip,sfc"},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
+
+static struct platform_driver rockchip_sfc_driver = {
+	.driver = {
+		.name	= "rockchip-sfc",
+		.of_match_table = rockchip_sfc_dt_ids,
+	},
+	.probe	= rockchip_sfc_probe,
+	.remove	= rockchip_sfc_remove,
+};
+module_platform_driver(rockchip_sfc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/2] mtd: spi-nor: Bindings for Rockchip serial flash controller
From: Shawn Lin @ 2016-11-11  9:16 UTC (permalink / raw)
  To: Rob Herring, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
	Shawn Lin
In-Reply-To: <1478855766-151673-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Add binding document for the Rockchip serial flash controller.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

 .../devicetree/bindings/mtd/rockchip-sfc.txt       | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/rockchip-sfc.txt

diff --git a/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt b/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
new file mode 100644
index 0000000..28430ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
@@ -0,0 +1,31 @@
+Rockchip Serial Flash Controller
+
+Required properties:
+- compatible : Should be
+		"rockchip,rk1108-sfc", "rockchip,sfc" for ROCKCHIP RK1108.
+- address-cells : Should be 1.
+- size-cells : Should be 0.
+- clocks: Must contain two entries for each entry in clock-names.
+- clock-names: Shall be "sfc" for the transfer-clock, and "hsfc" for
+		the peripheral clock.
+- interrupts : Should contain the interrupt for the device.
+- reg: Physical base address of the controller and length of memory mapped.
+
+Optional properties:
+- rockchip,sfc-no-dma: Indicate the controller doesn't support dma transfer.
+
+Example:
+nor_flash: sfc@301c0000 {
+	compatible = "rockchip,rk1108-sfc", "rockchip,sfc";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+	clock-names = "sfc", "hsfc";
+	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+	reg = <0x301c0000 0x1000>;
+	spi-nor@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <12000000>;
+		reg = <0>;
+	};
+};
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 0/2] Add rockchip serial flash controller support
From: Shawn Lin @ 2016-11-11  9:16 UTC (permalink / raw)
  To: Rob Herring, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
	Shawn Lin


This pathset is gonna support serial flash controller
, namely SFC, found on Rockchip RK1108 platform.

Feature:
(1) Support x1, x2, x4 data bits mode
(2) Support up to 4 chip select
(3) Support two independent clock domain: AHB clock and SPI clock
(4) Support DMA master up to 16KB/transfer

Test environment:
This patchset is testing on RK1108 evb boards with Winboud flash
(w25q256) and working find with PIO or DMA mode.

How-to:
Any rockchip guys who are interested in testing it could refer to
the following steps:
(1) enable CONFIG_MTD_M25P80
(2) enable CONFIG_SPI_ROCKCHIP_SFC
(3) enable CONFIG_MTD_CMDLINE_PARTS
(4) enable CONFIG_SQUASHFS
(4) CONFIG_CMDLINE="root=/dev/mtdblock2
	mtdparts=spi-nor:256k@0(loader)ro,8m(kernel)ro,7m(rootfs),-(freedisk)"
	Of course, you should check the partition layout if you modify it. Also
	you could pass it from your loader to the kernel's cmdline.
(5) Add dts support:
nor_flash: sfc@301c0000 {
	compatible = "rockchip,rk1108-sfc", "rockchip,sfc";
	#address-cells = <1>;
	#size-cells = <0>;
	clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
	clock-names = "sfc", "hsfc";
	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
	reg = <0x301c0000 0x1000>;
	/* If you want to use PIO mode, activate this */
	#rockchip,sfc-no-dma;
	spi-nor@0 {
		compatible = "jedec,spi-nor";
		spi-max-frequency = <12000000>;
		reg = <0>;
	}
};

please make sure your DT's mdtid matchs what you assgin to the
mdtparts(cmdline), namely they are both *spi-nor* here.

With enabling DBG for cmdlinepart.c, you could get following log and
boot kernel and rootfs successfully.

[    0.481420] rockchip-sfc 301c0000.sfc: w25q256 (32768 Kbytes)
[    0.481962] DEBUG-CMDLINE-PART: parsing
<256k@0(loader)ro,8m(kernel)ro,7m(rootfs)ro,-(freedisk)>
[    0.482897] DEBUG-CMDLINE-PART: partition 3: name
<freedisk>, offset ffffffffffffffff, size ffffffffffffffff, mask flags 0
[    0.484021] DEBUG-CMDLINE-PART: partition 2: name
<rootfs>, offset ffffffffffffffff, size 700000, mask flags 400
[    0.485066] DEBUG-CMDLINE-PART: partition 1: name
<kernel>, offset ffffffffffffffff, size 800000, mask flags 400
[    0.486108] DEBUG-CMDLINE-PART: partition 0: name
<loader>, offset 0, size 40000, mask flags 400
[    0.487152] DEBUG-CMDLINE-PART: mtdid=<spi-nor> num_parts=<4>
[    0.487827] 4 cmdlinepart partitions found on MTD device spi-nor
[    0.488370] Creating 4 MTD partitions on "spi-nor":
[    0.488826] 0x000000000000-0x000000040000 : "loader"
[    0.492340] 0x000000040000-0x000000840000 : "kernel"
[    0.495679] 0x000000840000-0x000000f40000 : "rootfs"
[    0.499241] 0x000000f40000-0x000002000000 : "freedisk"

[root@arm-linux]#
[root@arm-linux]#mount
/dev/root on / type squashfs (ro,relatime)
devtmpfs on /dev type devtmpfs
(rw,relatime,size=26124k,nr_inodes=6531,mode=755)
proc on /proc type proc (rw,relatime)
none on /tmp type ramfs (rw,relatime)
none on /var type ramfs (rw,relatime)
sysfs on /sys type sysfs (rw,relatime)
debug on /sys/kernel/debug type debugfs (rw,relatime)
none on /dev/pts type devpts (rw,relatime,mode=600,ptmxmode=000)



Shawn Lin (2):
  mtd: spi-nor: Bindings for Rockchip serial flash controller
  mtd: spi-nor: add rockchip serial flash controller driver

 .../devicetree/bindings/mtd/rockchip-sfc.txt       |  31 +
 MAINTAINERS                                        |   8 +
 drivers/mtd/spi-nor/Kconfig                        |   7 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/rockchip-sfc.c                 | 953 +++++++++++++++++++++
 5 files changed, 1000 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
 create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c

-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] backlight: arcxcnn: devicetree bindings for ArticSand devices
From: Lee Jones @ 2016-11-11  8:45 UTC (permalink / raw)
  To: Olimpiu Dejeu; +Cc: robh, linux-kernel, linux-fbdev, devicetree, jingoohan1
In-Reply-To: <1478792665-5859-1-git-send-email-olimpiu@arcticsand.com>

On Thu, 10 Nov 2016, Olimpiu Dejeu wrote:

> Resubmition of arcxcnn backlight driver addressing the naming convention
>  concerns raised by Rob H. Note that all the device tree properties are
>  determined by the board design or IC EPROM settings and are not intended
>  to be user adjustable.
> 
> Signed-off-by: Olimpiu Dejeu <olimpiu@arcticsand.com>

You've dropped Rob's Ack.

> ---
>  .../bindings/leds/backlight/arcxcnn_bl.txt         | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> new file mode 100644
> index 0000000..a7b6ff2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> @@ -0,0 +1,33 @@
> +Binding for ArcticSand arc2c0608 LED driver
> +
> +Required properties:
> +- compatible: should be "arc,arc2c0608"
> +- reg: slave address
> +
> +Optional properties:
> +- default-brightness: brightness value on boot, value from: 0-4095
> +- label: The name of the backlight device
> +			See Documentation/devicetree/bindings/leds/common.txt
> +- led-sources: List of enabled channels from 0 to 5.
> +			See Documentation/devicetree/bindings/leds/common.txt
> +
> +- arc,led-config-0: setting for register ILED_CONFIG_0
> +- arc,led-config-1: setting for register ILED_CONFIG_1
> +- arc,dim-freq: PWM mode frequence setting (bits [3:0] used)
> +- arc,comp-config: setting for register CONFIG_COMP
> +- arc,filter-config: setting for register FILTER_CONFIG
> +- arc,trim-config: setting for register IMAXTUNE
> +
> +Note: Optional properties not specified will default to values in IC EPROM
> +
> +Example:
> +
> +arc2c0608@30 {
> +	compatible = "arc,arc2c0608";
> +	reg = <0x30>;
> +	default-brightness = <500>;
> +	label = "lcd-backlight";
> +	linux,default-trigger = "backlight";
> +	led-sources = <0 1 2 5>;
> +};
> +

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

^ permalink raw reply

* Re: [PATCH v2 2/6] mfd: stm32-adc: Add support for stm32 ADC
From: Lee Jones @ 2016-11-11  8:42 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Fabrice Gasnier, kbuild-all, linux-iio, linux-arm-kernel,
	devicetree, linux-kernel, jic23, linux, robh+dt, mark.rutland,
	mcoquelin.stm32, alexandre.torgue, lars, knaack.h, pmeerw
In-Reply-To: <201611110537.S7AVN9F6%fengguang.wu@intel.com>

On Fri, 11 Nov 2016, kbuild test robot wrote:

> Hi Fabrice,
> 
> [auto build test ERROR on ljones-mfd/for-mfd-next]
> [also build test ERROR on v4.9-rc4 next-20161110]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Fabrice-Gasnier/Add-support-for-STM32-ADC/20161111-011922
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: s390-allmodconfig (attached as .config)
> compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=s390 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/mfd/stm32-adc-core: struct of_device_id is 200 bytes.  The last of 1 is:
>    0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x73 0x74 0x2c 0x73 0x74 0x6d 0x33 0x32 0x66 0x34 0x2d 0x61 0x64 0x63 0x2d 0x63 0x6f 0x72 0x65 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 
> >> FATAL: drivers/mfd/stm32-adc-core: struct of_device_id is not terminated with a NULL entry!
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Please fix this and re-submit.

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

^ permalink raw reply

* Re: [PATCH] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
From: Rameshwar Sahu @ 2016-11-11  8:06 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Olof Johansson, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Devicetree List, linux-arm,
	Don Dutile, Jon Masters, patches
In-Reply-To: <20161109164531.GA14630-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>

Hi Tejun,

On Wed, Nov 9, 2016 at 10:15 PM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hello,
>
> On Wed, Sep 14, 2016 at 04:15:00PM +0530, Rameshwar Sahu wrote:
>> > @@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device
>> > *pdev)
>> >                                 dev_warn(&pdev->dev, "%s: Error reading
>> > device info. Assume version1\n",
>> >                                         __func__);
>> >                                 version = XGENE_AHCI_V1;
>> > -                       } else if (info->valid & ACPI_VALID_CID) {
>> > -                               version = XGENE_AHCI_V2;
>
> Can you please explain this part a bit?  Everything else looks good to
> me.

Here we should not assume XGENE_AHCI_V2 always in case of having valid
_CID in ACPI table.
I need to remove this assumption because V1_1 has also valid _CID for
backward compatibly with v1.
>
> Thanks.
>
> --
> tejun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2 3/3] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
From: Mugunthan V N @ 2016-11-11  7:58 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA
  Cc: Dmitry Torokhov, Jonathan Cameron, Rob Herring, Mark Rutland,
	Lee Jones, Sekhar Nori, Vignesh R,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N
In-Reply-To: <20161110163515.27598-1-mugunthanvnm-l0cyMroinI0@public.gmane.org>

Increase ADC reference clock from 3MHz to 24MHz so that the
sampling rates goes up from 100K samples per second to 800K
samples per second on AM335x and AM437x SoC.

Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
---
 include/linux/mfd/ti_am335x_tscadc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index b9a53e013bff..f6c449b96ea5 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -137,7 +137,7 @@
 #define SEQ_STATUS BIT(5)
 #define CHARGE_STEP		0x11
 
-#define ADC_CLK			3000000
+#define ADC_CLK			24000000
 #define TOTAL_STEPS		16
 #define TOTAL_CHANNELS		8
 #define FIFO1_THRESHOLD		19
-- 
2.11.0.rc0.7.gbe5a750

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 2/3] Input: ti_am335x_tsc: Add support for ti,charge-delay-ns
From: Mugunthan V N @ 2016-11-11  7:58 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA
  Cc: Dmitry Torokhov, Jonathan Cameron, Rob Herring, Mark Rutland,
	Lee Jones, Sekhar Nori, Vignesh R,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N
In-Reply-To: <20161110163515.27598-1-mugunthanvnm-l0cyMroinI0@public.gmane.org>

ti,charge-delay will be deprecated as it represents number of
clock cycles and the DT entries are done in assumption of 3MHz
TSCADC clock, but clock can be set upto 24MHz. So driver add
support for ti,charge-delay-ns and do not drop support for
ti,charge-delay to support old dtbs and it will be assumed that
it is for 3MHz TSCADC clock and will be calculated as per current
clock speed.

Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
---
 drivers/input/touchscreen/ti_am335x_tsc.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 7953381d939a..104b3640f728 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -379,15 +379,30 @@ static int titsc_parse_dt(struct platform_device *pdev,
 		ts_dev->coordinate_readouts = 5;
 	}
 
-	err = of_property_read_u32(node, "ti,charge-delay",
+	err = of_property_read_u32(node, "ti,charge-delay-ns",
 				   &ts_dev->charge_delay);
-	/*
-	 * If ti,charge-delay value is not specified, then use
-	 * CHARGEDLY_OPENDLY as the default value.
-	 */
-	if (err < 0) {
-		ts_dev->charge_delay = CHARGEDLY_OPENDLY;
-		dev_warn(&pdev->dev, "ti,charge-delay not specified\n");
+	if (err >= 0) {
+		u64 charge_delay = ts_dev->charge_delay;
+
+		charge_delay *= ADC_CLK;
+		do_div(charge_delay, 1E9);
+		ts_dev->charge_delay = (u32)charge_delay;
+	} else {
+		err = of_property_read_u32(node, "ti,charge-delay",
+					   &ts_dev->charge_delay);
+		/*
+		 * If ti,charge-delay value is not specified, then use
+		 * CHARGEDLY_OPENDLY as the default value.
+		 */
+		if (err < 0) {
+			ts_dev->charge_delay = CHARGEDLY_OPENDLY;
+			dev_warn(&pdev->dev, "ti,charge-delay not specified\n");
+		}
+		/*
+		 * ti,charge-delay is specified with referrence to 3MHz,
+		 * so convert it to in referrence to current clock
+		 */
+		ts_dev->charge_delay *= ADC_CLK / 3000000;
 	}
 
 	return of_property_read_u32_array(node, "ti,wire-config",
-- 
2.11.0.rc0.7.gbe5a750

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-11  7:48 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Sudeep Holla, Rob Herring, Neil Armstrong,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAOesGMj3TYLkvfGbq6BKqr+9i6mjArjRPuqeAN1-rGO2OhoSyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 10/11/16 19:03, Olof Johansson wrote:
> On Thu, Nov 10, 2016 at 6:34 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
>>
>>
>> On 10/11/16 14:12, Rob Herring wrote:
>>>
>>> On Thu, Nov 10, 2016 at 4:26 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 10/11/16 01:22, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Wed, Nov 02, 2016 at 10:52:09PM -0600, Sudeep Holla wrote:
>>>>>>
>>>>>>
>>>>>> This patch adds specific compatible to support legacy SCPI protocol.
>>>>>>
>>>>>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
>>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> index d1882c4540d0..ebd03fc93135 100644
>>>>>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>>> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power
>>>>>> operations.
>>>>>>
>>>>>>  Required properties:
>>>>>>
>>>>>> -- compatible : should be "arm,scpi"
>>>>>> +- compatible : should be
>>>>>> +       * "arm,scpi" : For implementations complying to SCPI v1.0 or
>>>>>> above
>>>>>> +       * "arm,legacy-scpi" : For implementations complying pre SCPI
>>>>>> v1.0
>>>>>
>>>>>
>>>>>
>>>>> I'd prefer that we explicitly enumerate the old versions. Are there
>>>>> many?
>>>>>
>>>>
>>>> I understand your concern, but this legacy SCPI protocol was not
>>>> officially released. It was just WIP which vendors picked up from very
>>>> early releases. Since they are not numbered, it's hard to have specific
>>>> compatibles with different versions until v1.0. That's one of the reason
>>>> to retain platform specific compatible so that we can add any quirks
>>>> based on them if needed.
>>>>
>>>> I will probably add these information in the commit log so that it's
>>>> clear why we can't do version based compatible.
>>>
>>>
>>> This is exactly my point. By enumerate, I meant having platform
>>> specific compatibles. Having "arm,legacy-scpi" is pointless because
>>> who knows what version they followed and they may all be different.
>>>
>>
>> OK, but IIUC Olof's concern wanted a generic one along with the platform
>> specific compatible which kind of makes sense as so far we have seen
>> some commonality between Amlogic and Rockchip.
>>
>> E.g. Amlogic follows most of the legacy protocol though it deviates in
>> couple of things which we can handle with platform specific compatible
>> (in the following patch in the series). When another user(Rockchip ?)
>> make use of this legacy protocol, we can start using those platform
>> specific compatible for deviations only.
>>
>> Is that not acceptable ?
>
> If there's no shared legacy feature set, then it's probably less
> useful to have a shared less precise compatible value.
>

There is and will be some shared feature set for sure. At the least the
standard command set will be shared.

> What the main point I was trying to get across was that we shouldn't
> expand the generic binding with per-vendor compatible fields, instead
> we should have those as extensions on the side.
>

Yes I get the point. We will have per-vendor compatibles for handle the
deviations but generic one to handle the shared set.

> I'm also a little apprehensive of using "legacy", it goes in the same
> bucket as "misc". At some point 1.0 will be legacy too, etc.
>

True and I agree, how about "arm,scpi-pre-1.0" instead ?

--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 00/23] Support qcom's HSIC USB and rewrite USB2 HS support
From: Peter Chen @ 2016-11-11  7:40 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi, Heikki Krogerus,
	Arnd Bergmann, Neil Armstrong,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bjorn Andersson,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Peter Chen,
	Greg Kroah-Hartman, Andy Gross, Ivan T. Ivanov,
	Kishon Vijay Abraham I,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <147682389932.20076.4186408979552866325@sboyd-linaro>

On Tue, Oct 18, 2016 at 01:51:39PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-10-18 02:31:40)
> > On Mon, Oct 17, 2016 at 06:56:13PM -0700, Stephen Boyd wrote:
> > > I've also sent separate patches for other minor pieces to make this
> > > all work. The full tree can be found here[2], hacks and all to get
> > > things working. I've tested this on the db410c, apq8074 dragonboard,
> > > and ifc6410 with configfs gadgets and otg cables.
> > > 
> > > Patches based on v4.8-rc1
> 
> Oops should be v4.9-rc1 here.
> 
> > > 
> > > Changes from v4:
> > >  * Picked up Acks from Rob
> > >  * Updated HS phy init sequence DT property to restrict it to offsets
> > 
> > I remembered that you got all my acks for chipidea patches, right? I did
> > not check for this series.
> 
> Sorry I've added in one more patch
> 
>    usb: chipidea: Emulate OTGSC interrupt enable path
> 
> to fix extcon interrupt emulation even further.
> 
> > 
> > Besides, the patch "gpu: Remove depends on RESET_CONTROLLER when not a
> > provider" [1]  still not be accepted, I need this patch to be merged
> > first, then apply your chipidea part, otherwise, there is a building
> > warning.
> > 
> > [1] https://patchwork.kernel.org/patch/9322583/
> 
> Yes, I'm going to resend that patch now. I hope that David will apply it
> for -rc2.

Stephen, just a mind. I have rebased Greg's usb-next tree (v4.9-rc3 on
it), your GPU fix is still not there.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Antti Palosaari @ 2016-11-11  7:16 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab, hverkuil,
	sakari.ailus
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-3-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

Hello

On 11/09/2016 05:44 PM, Ramesh Shanmugasundaram wrote:

> +static int max2175_set_lo_freq(struct max2175 *ctx, u64 lo_freq)
> +{
> +	u64 scaled_lo_freq, scaled_npf, scaled_integer, scaled_fraction;
> +	u32 frac_desired, int_desired, lo_mult = 1;
> +	const u32 scale_factor = 1000000U;
> +	u8 loband_bits = 0, vcodiv_bits = 0;
> +	enum max2175_band band;
> +	int ret;
> +
> +	/* Scale to larger number for precision */
> +	scaled_lo_freq = lo_freq * scale_factor * 100;
> +	band = max2175_read_bits(ctx, 5, 1, 0);
> +
> +	mxm_dbg(ctx, "set_lo_freq: scaled lo_freq %llu lo_freq %llu band %d\n",
> +		scaled_lo_freq, lo_freq, band);
> +
> +	switch (band) {
> +	case MAX2175_BAND_AM:
> +		if (max2175_read_bit(ctx, 5, 7) == 0)
> +			lo_mult = 16;

else is lo_mult = 1. No idea if it is correct, but sounds very small 
output divider for low freq like am band. And on the other-hand local 
oscillator output divider, which I expect this to be, is usually 2 or more.

> +		break;
> +	case MAX2175_BAND_FM:
> +		if (lo_freq <= 74700000) {
> +			lo_mult = 16;

No meaning as you set it later 8.

> +		} else if (lo_freq > 74700000 && lo_freq <= 110000000) {
> +			loband_bits = 1;
> +		} else {
> +			loband_bits = 1;
> +			vcodiv_bits = 3;
> +		}
> +		lo_mult = 8;
> +		break;
> +	case MAX2175_BAND_VHF:
> +		if (lo_freq <= 210000000) {
> +			loband_bits = 2;
> +			vcodiv_bits = 2;
> +		} else {
> +			loband_bits = 2;
> +			vcodiv_bits = 1;
> +		}
> +		lo_mult = 4;
> +		break;
> +	default:
> +		loband_bits = 3;
> +		vcodiv_bits = 2;
> +		lo_mult = 2;
> +		break;
> +	}
> +
> +	if (band == MAX2175_BAND_L)
> +		scaled_npf = div_u64(div_u64(scaled_lo_freq, ctx->xtal_freq),
> +				     lo_mult);
> +	else
> +		scaled_npf = div_u64(scaled_lo_freq, ctx->xtal_freq) * lo_mult;
> +
> +	scaled_npf = div_u64(scaled_npf, 100);
> +	scaled_integer = div_u64(scaled_npf, scale_factor) * scale_factor;
> +	int_desired = div_u64(scaled_npf, scale_factor);
> +	scaled_fraction = scaled_npf - scaled_integer;
> +	frac_desired = div_u64(scaled_fraction << 20, scale_factor);
> +
> +	/* Check CSM is not busy */
> +	ret = max2175_poll_csm_ready(ctx);
> +	if (ret)
> +		return ret;
> +
> +	mxm_dbg(ctx, "loband %u vcodiv %u lo_mult %u scaled_npf %llu\n",
> +		loband_bits, vcodiv_bits, lo_mult, scaled_npf);
> +	mxm_dbg(ctx, "scaled int %llu frac %llu desired int %u frac %u\n",
> +		scaled_integer, scaled_fraction, int_desired, frac_desired);
> +
> +	/* Write the calculated values to the appropriate registers */
> +	max2175_write(ctx, 1, int_desired);
> +	max2175_write_bits(ctx, 2, 3, 0, (frac_desired >> 16) & 0xf);
> +	max2175_write(ctx, 3, frac_desired >> 8);
> +	max2175_write(ctx, 4, frac_desired);
> +	max2175_write_bits(ctx, 5, 3, 2, loband_bits);
> +	max2175_write_bits(ctx, 6, 7, 6, vcodiv_bits);
> +	return ret;
> +}

That synthesizer config is hard to understand. It seems to be 
fractional-N, with configurable N, K and output divider - like a school 
book example.

               +----------------------------+
               v                            |
      Fref   +----+     +-------+         +------+
     ------> | PD | --> |  VCO  | ------> | /N.F |
             +----+     +-------+         +------+
                          |
                          |
                          v
                        +-------+  Fout
                        | /Rout | ------>
                        +-------+

I made following look-up table in order to understand it:

band      lo freq band vcodiv div_out
   AM  <  50000000    0      0      16 // reg 5 bit 7 ?
   FM  <  74700000    0      0      16
   FM  < 110000000    1      0       8
   FM  < 160000000    1      3       8
  VHF  < 210000000    2      2       4
  VHF  < 600000000    2      1       4
    L  <2000000000    3      2       2

"vcodiv" looks unrelated to synth calculation, dunno what it is.

One which makes calculation very complex looking is that it is based of 
floating point calculus. On integer mathematics you should replace 
fractional part with fractional modulus (usually letter "K" is used for 
fractional modulus on PLL calc).

So that ends up something like:
1) select suitable lo output divider from desired output frequency
2) calculate vco frequency
3) convert vco frequency to N and K
* N = Fvco/Fref
* K = Fvco%Fref
4) convert K to control word (looks like << 20)
5) program values

Result should be calculus without scaling.

regards
Antti


-- 
http://palosaari.fi/

^ permalink raw reply

* Re: [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Archit Taneja @ 2016-11-11  6:02 UTC (permalink / raw)
  To: Enric Balletbo Serra, Jitao Shi, djkurtz-F7+t8E8rja9g9hUCZPvPmw
  Cc: David Airlie, Thierry Reding, Matthias Brugger, Mark Rutland,
	stonea168-9Onoh4P/yGk, dri-devel, Andy Yan, Ajay Kumar,
	Vincent Palatin, cawa cheng, bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w,
	CK HU, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sascha Hauer,
	Pawel Moll, Ian Campbell, Inki Dae, Rob Herring, ARM/Mediatek 
In-Reply-To: <CAFqH_52A4=GubRjPa8X1=STExXFZy2HdoASEGUBYwqGt0BEFcw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Jitao,

I couldn't locate the original mail, so posting on this thread instead.
Some comments below.

On 11/10/2016 10:09 PM, Enric Balletbo Serra wrote:
> Hi Jitao,
>
> 2016-08-27 8:44 GMT+02:00 Jitao Shi <jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>:
>> This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
>>
>> Signed-off-by: Jitao Shi <jitao.shi-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> Changes since v16:
>>  - Disable ps8640 DSI MCS Function.
>>  - Rename gpios name more clearly.
>>  - Tune the ps8640 power on sequence.
>>
>> Changes since v15:
>>  - Drop drm_connector_(un)register calls from parade ps8640.
>>    The main DRM driver mtk_drm_drv now calls
>>    drm_connector_register_all() after drm_dev_register() in the
>>    mtk_drm_bind() function. That function should iterate over all
>>    connectors and call drm_connector_register() for each of them.
>>    So, remove drm_connector_(un)register calls from parade ps8640.
>>
>> Changes since v14:
>>  - update copyright info.
>>  - change bridge_to_ps8640 and connector_to_ps8640 to inline function.
>>  - fix some coding style.
>>  - use sizeof as array counter.
>>  - use drm_get_edid when read edid.
>>  - add mutex when firmware updating.
>>
>> Changes since v13:
>>  - add const on data, ps8640_write_bytes(struct i2c_client *client, const u8 *data, u16 data_len)
>>  - fix PAGE2_SW_REST tyro.
>>  - move the buf[3] init to entrance of the function.
>>
>> Changes since v12:
>>  - fix hw_chip_id build warning
>>
>> Changes since v11:
>>  - Remove depends on I2C, add DRM depends
>>  - Reuse ps8640_write_bytes() in ps8640_write_byte()
>>  - Use timer check for polling like the routines in <linux/iopoll.h>
>>  - Fix no drm_connector_unregister/drm_connector_cleanup when ps8640_bridge_attach fail
>>  - Check the ps8640 hardware id in ps8640_validate_firmware
>>  - Remove fw_version check
>>  - Move ps8640_validate_firmware before ps8640_enter_bl
>>  - Add ddc_i2c unregister when probe fail and ps8640_remove
>> ---
>>  drivers/gpu/drm/bridge/Kconfig         |   12 +
>>  drivers/gpu/drm/bridge/Makefile        |    1 +
>>  drivers/gpu/drm/bridge/parade-ps8640.c | 1077 ++++++++++++++++++++++++++++++++
>>  3 files changed, 1090 insertions(+)
>>  create mode 100644 drivers/gpu/drm/bridge/parade-ps8640.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index b590e67..c59d043 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -50,6 +50,18 @@ config DRM_PARADE_PS8622
>>         ---help---
>>           Parade eDP-LVDS bridge chip driver.
>>
>> +config DRM_PARADE_PS8640
>> +       tristate "Parade PS8640 MIPI DSI to eDP Converter"
>> +       depends on DRM
>> +       depends on OF
>> +       select DRM_KMS_HELPER
>> +       select DRM_MIPI_DSI
>> +       select DRM_PANEL
>> +       ---help---
>> +         Choose this option if you have PS8640 for display
>> +         The PS8640 is a high-performance and low-power
>> +         MIPI DSI to eDP converter
>> +
>>  config DRM_SII902X
>>         tristate "Silicon Image sii902x RGB/HDMI bridge"
>>         depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index efdb07e..3360537 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>> +obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
>>  obj-$(CONFIG_DRM_SII902X) += sii902x.o
>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
>> new file mode 100644
>> index 0000000..7d67431
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
>> @@ -0,0 +1,1077 @@
>> +/*
>> + * Copyright (c) 2016 MediaTek Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/firmware.h>
>> +#include <linux/gpio.h>

Not needed.

>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>

The above 2 aren't needed.

>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <asm/unaligned.h>
>> +#include <drm/drm_panel.h>
>> +
>> +#include <drmP.h>
>> +#include <drm_atomic_helper.h>
>> +#include <drm_crtc_helper.h>
>> +#include <drm_crtc.h>

Not needed.

>> +#include <drm_edid.h>
>> +#include <drm_mipi_dsi.h>
>> +
>> +#define PAGE1_VSTART           0x6b
>> +#define PAGE2_SPI_CFG3         0x82
>> +#define I2C_TO_SPI_RESET       0x20
>> +#define PAGE2_ROMADD_BYTE1     0x8e
>> +#define PAGE2_ROMADD_BYTE2     0x8f
>> +#define PAGE2_SWSPI_WDATA      0x90
>> +#define PAGE2_SWSPI_RDATA      0x91
>> +#define PAGE2_SWSPI_LEN                0x92
>> +#define PAGE2_SWSPI_CTL                0x93
>> +#define TRIGGER_NO_READBACK    0x05
>> +#define TRIGGER_READBACK       0x01
>> +#define PAGE2_SPI_STATUS       0x9e
>> +#define SPI_READY              0x0c
>> +#define PAGE2_GPIO_L           0xa6
>> +#define PAGE2_GPIO_H           0xa7
>> +#define PS_GPIO9               BIT(1)
>> +#define PAGE2_IROM_CTRL                0xb0
>> +#define IROM_ENABLE            0xc0
>> +#define IROM_DISABLE           0x80
>> +#define PAGE2_SW_RESET         0xbc
>> +#define SPI_SW_RESET           BIT(7)
>> +#define MPU_SW_RESET           BIT(6)
>> +#define PAGE2_ENCTLSPI_WR      0xda
>> +#define PAGE2_I2C_BYPASS       0xea
>> +#define I2C_BYPASS_EN          0xd0
>> +#define PAGE2_MCS_EN           0xf3
>> +#define MCS_EN                 BIT(0)
>> +#define PAGE3_SET_ADD          0xfe
>> +#define PAGE3_SET_VAL          0xff
>> +#define VDO_CTL_ADD            0x13
>> +#define VDO_DIS                        0x18
>> +#define VDO_EN                 0x1c
>> +#define PAGE4_REV_L            0xf0
>> +#define PAGE4_REV_H            0xf1
>> +#define PAGE4_CHIP_L           0xf2
>> +#define PAGE4_CHIP_H           0xf3
>> +
>> +/* Firmware */
>> +#define PS_FW_NAME             "ps864x_fw.bin"
>> +
>
> About the firmware discussion I think that if you want to maintain the
> upgrade firmware thing you should also include this patch in the
> series.
>
>  https://chromium-review.googlesource.com/#/c/317221/
>
> Otherwise, if this is not really needed I think that remove this from
> the driver is the best. Just an opinion, this is something the
> maintainer should decide.

Was there a conclusion on this? As Daniel Kurtz suggested, can we drop
the update firmware stuff for now and try to get the functional part
for 4.10?

>
>> +#define FW_CHIP_ID_OFFSET      0
>> +#define FW_VERSION_OFFSET      2
>> +#define EDID_I2C_ADDR          0x50
>> +
>> +#define WRITE_STATUS_REG_CMD   0x01
>> +#define READ_STATUS_REG_CMD    0x05
>> +#define BUSY                   BIT(0)
>> +#define CLEAR_ALL_PROTECT      0x00
>> +#define BLK_PROTECT_BITS       0x0c
>> +#define STATUS_REG_PROTECT     BIT(7)
>> +#define WRITE_ENABLE_CMD       0x06
>> +#define CHIP_ERASE_CMD         0xc7
>> +#define MAX_DEVS               0x8
>> +
>> +struct ps8640_info {
>> +       u8 family_id;
>> +       u8 variant_id;
>> +       u16 version;
>> +};
>> +
>> +struct ps8640 {
>> +       struct drm_connector connector;
>> +       struct drm_bridge bridge;
>> +       struct edid *edid;
>> +       struct mipi_dsi_device dsi;
>> +       struct i2c_client *page[MAX_DEVS];
>> +       struct i2c_client *ddc_i2c;
>> +       struct regulator_bulk_data supplies[2];
>> +       struct drm_panel *panel;
>> +       struct gpio_desc *gpio_reset;
>> +       struct gpio_desc *gpio_power_down;
>> +       struct gpio_desc *gpio_mode_sel;
>> +       bool enabled;
>> +
>> +       /* firmware file info */
>> +       struct ps8640_info info;
>> +       bool in_fw_update;
>> +       /* for firmware update protect */
>> +       struct mutex fw_mutex;
>> +};
>> +
>> +static const u8 enc_ctrl_code[6] = { 0xaa, 0x55, 0x50, 0x41, 0x52, 0x44 };
>> +static const u8 hw_chip_id[4] = { 0x00, 0x0a, 0x00, 0x30 };
>> +
>> +static inline struct ps8640 *bridge_to_ps8640(struct drm_bridge *e)
>> +{
>> +       return container_of(e, struct ps8640, bridge);
>> +}
>> +
>> +static inline struct ps8640 *connector_to_ps8640(struct drm_connector *e)
>> +{
>> +       return container_of(e, struct ps8640, connector);
>> +}
>> +
>> +static int ps8640_read(struct i2c_client *client, u8 reg, u8 *data,
>> +                      u16 data_len)
>> +{
>> +       int ret;
>> +       struct i2c_msg msgs[] = {
>> +               {
>> +                .addr = client->addr,
>> +                .flags = 0,
>> +                .len = 1,
>> +                .buf = &reg,
>> +               },
>> +               {
>> +                .addr = client->addr,
>> +                .flags = I2C_M_RD,
>> +                .len = data_len,
>> +                .buf = data,
>> +               }
>> +       };
>> +
>> +       ret = i2c_transfer(client->adapter, msgs, 2);
>> +
>> +       if (ret == 2)
>> +               return 0;
>> +       if (ret < 0)
>> +               return ret;
>> +       else
>> +               return -EIO;
>> +}
>> +
>> +static int ps8640_write_bytes(struct i2c_client *client, const u8 *data,
>> +                             u16 data_len)
>> +{
>> +       int ret;
>> +       struct i2c_msg msg;
>> +
>> +       msg.addr = client->addr;
>> +       msg.flags = 0;
>> +       msg.len = data_len;
>> +       msg.buf = (u8 *)data;
>> +
>> +       ret = i2c_transfer(client->adapter, &msg, 1);
>> +       if (ret == 1)
>> +               return 0;
>> +       if (ret < 0)
>> +               return ret;
>> +       else
>> +               return -EIO;
>> +}
>> +
>> +static int ps8640_write_byte(struct i2c_client *client, u8 reg,  u8 data)
>> +{
>> +       u8 buf[] = { reg, data };
>> +
>> +       return ps8640_write_bytes(client, buf, sizeof(buf));
>> +}
>> +
>> +static void ps8640_get_mcu_fw_version(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[5];
>> +       u8 fw_ver[2];
>> +
>> +       ps8640_read(client, 0x4, fw_ver, sizeof(fw_ver));
>> +       ps_bridge->info.version = (fw_ver[0] << 8) | fw_ver[1];
>> +
>> +       DRM_INFO_ONCE("ps8640 rom fw version %d.%d\n", fw_ver[0], fw_ver[1]);
>> +}
>> +
>> +static int ps8640_bridge_unmute(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[3];
>> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_EN };
>> +
>> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static int ps8640_bridge_mute(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[3];
>> +       u8 vdo_ctrl_buf[3] = { PAGE3_SET_ADD, VDO_CTL_ADD, VDO_DIS };
>> +
>> +       return ps8640_write_bytes(client, vdo_ctrl_buf, sizeof(vdo_ctrl_buf));
>> +}
>> +
>> +static void ps8640_pre_enable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       struct i2c_client *page1 = ps_bridge->page[1];

It's a bit hard to follow what page[3] or page[5] means without going to the
bottom and reading the dummy devices comment. It would be nice to have some
macros here.

>> +       int err;
>> +       u8 set_vdo_done, mcs_en, vstart;
>> +       ktime_t timeout;
>> +
>> +       if (ps_bridge->in_fw_update)
>> +               return;
>> +
>> +       if (ps_bridge->enabled)
>> +               return;
>> +
>> +       err = drm_panel_prepare(ps_bridge->panel);
>> +       if (err < 0) {
>> +               DRM_ERROR("failed to prepare panel: %d\n", err);
>> +               return;
>> +       }
>> +
>> +       err = regulator_bulk_enable(ARRAY_SIZE(ps_bridge->supplies),
>> +                                   ps_bridge->supplies);
>> +       if (err < 0) {
>> +               DRM_ERROR("cannot enable regulators %d\n", err);
>> +               goto err_panel_unprepare;
>> +       }
>> +
>> +       gpiod_set_value(ps_bridge->gpio_power_down, 1);
>> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
>> +       usleep_range(2000, 2500);
>> +       gpiod_set_value(ps_bridge->gpio_reset, 1);
>> +
>> +       /*
>> +        * Wait for the ps8640 embed mcu ready
>> +        * First wait 200ms and then check the mcu ready flag every 20ms
>> +        */
>> +       msleep(200);
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 200);
>> +       for (;;) {
>> +               err = ps8640_read(client, PAGE2_GPIO_H, &set_vdo_done, 1);
>> +               if (err < 0) {
>> +                       DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", err);
>> +                       goto err_regulators_disable;
>> +               }
>> +               if ((set_vdo_done & PS_GPIO9) == PS_GPIO9)
>> +                       break;
>> +               if (ktime_compare(ktime_get(), timeout) > 0)
>> +                       break;
>> +               msleep(20);
>> +       }
>> +
>> +       msleep(50);
>> +
>> +       ps8640_read(page1, PAGE1_VSTART, &vstart, 1);
>> +       DRM_INFO("PS8640 PAGE1.0x6B = 0x%x\n", vstart);
>> +
>> +       /**
>> +        * The Manufacturer Command Set (MCS) is a device dependent interface
>> +        * intended for factory programming of the display module default
>> +        * parameters. Once the display module is configured, the MCS shall be
>> +        * disabled by the manufacturer. Once disabled, all MCS commands are
>> +        * ignored by the display interface.
>> +        */
>> +       ps8640_read(client, PAGE2_MCS_EN, &mcs_en, 1);
>> +       ps8640_write_byte(client, PAGE2_MCS_EN, mcs_en & ~MCS_EN);
>> +
>> +       if (ps_bridge->info.version == 0)
>> +               ps8640_get_mcu_fw_version(ps_bridge);
>> +
>> +       err = ps8640_bridge_unmute(ps_bridge);
>> +       if (err)
>> +               DRM_ERROR("failed to enable unmutevideo: %d\n", err);
>> +       /* Switch access edp panel's edid through i2c */
>> +       ps8640_write_byte(client, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
>> +       ps_bridge->enabled = true;
>> +
>> +       return;
>> +
>> +err_regulators_disable:
>> +       regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> +                              ps_bridge->supplies);
>> +err_panel_unprepare:
>> +       drm_panel_unprepare(ps_bridge->panel);
>> +}
>> +
>> +static void ps8640_enable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       err = drm_panel_enable(ps_bridge->panel);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to enable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_disable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       err = drm_panel_disable(ps_bridge->panel);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to disable panel: %d\n", err);
>> +}
>> +
>> +static void ps8640_post_disable(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       int err;
>> +
>> +       if (ps_bridge->in_fw_update)
>> +               return;
>> +
>> +       if (!ps_bridge->enabled)
>> +               return;
>> +
>> +       ps_bridge->enabled = false;
>> +
>> +       err = ps8640_bridge_mute(ps_bridge);
>> +       if (err < 0)
>> +               DRM_ERROR("failed to unmutevideo: %d\n", err);
>> +
>> +       gpiod_set_value(ps_bridge->gpio_reset, 0);
>> +       gpiod_set_value(ps_bridge->gpio_power_down, 0);
>> +       err = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
>> +                                    ps_bridge->supplies);
>> +       if (err < 0)
>> +               DRM_ERROR("cannot disable regulators %d\n", err);
>> +
>> +       err = drm_panel_unprepare(ps_bridge->panel);
>> +       if (err)
>> +               DRM_ERROR("failed to unprepare panel: %d\n", err);
>> +}
>> +
>> +static int ps8640_get_modes(struct drm_connector *connector)
>> +{
>> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> +       struct edid *edid;
>> +       int num_modes = 0;
>> +       bool power_off;
>> +
>> +       if (ps_bridge->edid)
>> +               return drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> +       power_off = !ps_bridge->enabled;
>> +       ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> +       edid = drm_get_edid(connector, ps_bridge->ddc_i2c->adapter);

See comments related to this in ps8640_probe.

>> +       if (!edid)
>> +               goto out;
>> +
>> +       ps_bridge->edid = edid;
>> +       drm_mode_connector_update_edid_property(connector, ps_bridge->edid);
>> +       num_modes = drm_add_edid_modes(connector, ps_bridge->edid);
>> +
>> +out:
>> +       if (power_off)
>> +               ps8640_post_disable(&ps_bridge->bridge);
>> +
>> +       return num_modes;
>> +}
>> +
>> +static struct drm_encoder *ps8640_best_encoder(struct drm_connector *connector)
>> +{
>> +       struct ps8640 *ps_bridge = connector_to_ps8640(connector);
>> +
>> +       return ps_bridge->bridge.encoder;
>> +}

We can drop the above func.

>> +
>> +static const struct drm_connector_helper_funcs ps8640_connector_helper_funcs = {
>> +       .get_modes = ps8640_get_modes,
>> +       .best_encoder = ps8640_best_encoder,
>> +};
>> +
>> +static enum drm_connector_status ps8640_detect(struct drm_connector *connector,
>> +                                              bool force)
>> +{
>> +       return connector_status_connected;
>> +}
>> +
>> +static const struct drm_connector_funcs ps8640_connector_funcs = {
>> +       .dpms = drm_atomic_helper_connector_dpms,
>> +       .fill_modes = drm_helper_probe_single_connector_modes,
>> +       .detect = ps8640_detect,
>> +       .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 ps8640_bridge_attach(struct drm_bridge *bridge)
>> +{
>> +       struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
>> +       struct device *dev = &ps_bridge->page[0]->dev;
>> +       struct device_node *port, *in_ep;
>> +       struct device_node *dsi_node = NULL;
>> +       struct mipi_dsi_host *host = NULL;
>> +       int ret;
>> +
>> +       ret = drm_connector_init(bridge->dev, &ps_bridge->connector,
>> +                                &ps8640_connector_funcs,
>> +                                DRM_MODE_CONNECTOR_eDP);
>> +
>> +       if (ret) {
>> +               DRM_ERROR("Failed to initialize connector with drm: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       drm_connector_helper_add(&ps_bridge->connector,
>> +                                &ps8640_connector_helper_funcs);
>> +
>> +       ps_bridge->connector.dpms = DRM_MODE_DPMS_ON;
>> +       drm_mode_connector_attach_encoder(&ps_bridge->connector,
>> +                                         bridge->encoder);
>> +
>> +       if (ps_bridge->panel)
>> +               drm_panel_attach(ps_bridge->panel, &ps_bridge->connector);
>> +
>> +       /* port@0 is ps8640 dsi input port */
>> +       port = of_graph_get_port_by_id(dev->of_node, 0);
>> +       if (port) {
>> +               in_ep = of_get_child_by_name(port, "endpoint");
>> +               of_node_put(port);

The above 2 funcs can be done by a single func: of_graph_get_endpoint_by_regs().

>> +               if (in_ep) {
>> +                       dsi_node = of_graph_get_remote_port_parent(in_ep);
>> +                       of_node_put(in_ep);
>> +               }
>> +       }
>> +       if (dsi_node) {
>> +               host = of_find_mipi_dsi_host_by_node(dsi_node);
>> +               of_node_put(dsi_node);
>> +               if (!host) {
>> +                       ret = -ENODEV;
>> +                       goto err;
>> +               }
>> +       }
>> +

We haven't created a DSI device for this yet. Don't we need to call
mipi_dsi_device_register_full() here?

>> +       ps_bridge->dsi.host = host;

The code above proceeds even if we don't find a dsi host. In that
case, the host would be a NULL pointer. We shouldn't call
mipi_dsi_attach() with a NULL host. We should have returned earlier with
an error.

>> +       ps_bridge->dsi.mode_flags = MIPI_DSI_MODE_VIDEO |
>> +                                    MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
>> +       ps_bridge->dsi.format = MIPI_DSI_FMT_RGB888;
>> +       ps_bridge->dsi.lanes = 4;
>> +       ret = mipi_dsi_attach(&ps_bridge->dsi);
>> +       if (ret)
>> +               goto err;
>> +
>> +       return 0;
>> +err:
>> +       if (ps_bridge->panel)
>> +               drm_panel_detach(ps_bridge->panel);
>> +       drm_connector_cleanup(&ps_bridge->connector);
>> +       return ret;
>> +}
>> +
>> +static const struct drm_bridge_funcs ps8640_bridge_funcs = {
>> +       .attach = ps8640_bridge_attach,
>> +       .disable = ps8640_disable,
>> +       .post_disable = ps8640_post_disable,
>> +       .pre_enable = ps8640_pre_enable,
>> +       .enable = ps8640_enable,
>> +};
>> +
>> +/* Firmware Version is returned as Major.Minor */
>> +static ssize_t ps8640_fw_version_show(struct device *dev,
>> +                                     struct device_attribute *attr, char *buf)
>> +{
>> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> +       struct ps8640_info *info = &ps_bridge->info;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "%u.%u\n", info->version >> 8,
>> +                        info->version & 0xff);
>> +}
>> +
>> +/* Hardware Version is returned as FamilyID.VariantID */
>> +static ssize_t ps8640_hw_version_show(struct device *dev,
>> +                                     struct device_attribute *attr, char *buf)
>> +{
>> +       struct ps8640 *ps_bridge = dev_get_drvdata(dev);
>> +       struct ps8640_info *info = &ps_bridge->info;
>> +
>> +       return scnprintf(buf, PAGE_SIZE, "ps%u.%u\n", info->family_id,
>> +                        info->variant_id);
>> +}
>> +
>> +static int ps8640_spi_send_cmd(struct ps8640 *ps_bridge, u8 *cmd, u8 cmd_len)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 i, buf[3] = { PAGE2_SWSPI_LEN, cmd_len - 1, TRIGGER_NO_READBACK };
>> +       int ret;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       if (ret)
>> +               goto err;
>> +
>> +       /* write command in write port */
>> +       for (i = 0; i < cmd_len; i++) {
>> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA, cmd[i]);
>> +               if (ret)
>> +                       goto err_irom_disable;
>> +       }
>> +
>> +       ret = ps8640_write_bytes(client, buf, sizeof(buf));
>> +       if (ret)
>> +               goto err_irom_disable;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +       if (ret)
>> +               goto err;
>> +
>> +       return 0;
>> +err_irom_disable:
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +err:
>> +       dev_err(&client->dev, "send command err: %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_wait_spi_ready(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 spi_rdy_st;
>> +       ktime_t timeout;
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 200);
>> +       for (;;) {
>> +               ps8640_read(client, PAGE2_SPI_STATUS, &spi_rdy_st, 1);
>> +               if ((spi_rdy_st & SPI_READY) != SPI_READY)
>> +                       break;
>> +
>> +               if (ktime_compare(ktime_get(), timeout) > 0) {
>> +                       dev_err(&client->dev, "wait spi ready timeout\n");
>> +                       return -EBUSY;
>> +               }
>> +
>> +               msleep(20);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_wait_spi_nobusy(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       u8 spi_status, buf[3] = { PAGE2_SWSPI_LEN, 0, TRIGGER_READBACK };
>> +       int ret;
>> +       ktime_t timeout;
>> +
>> +       timeout = ktime_add_ms(ktime_get(), 500);
>> +       for (;;) {
>> +               /* 0x05 RDSR; Read-Status-Register */
>> +               ret = ps8640_write_byte(client, PAGE2_SWSPI_WDATA,
>> +                                       READ_STATUS_REG_CMD);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               ret = ps8640_write_bytes(client, buf, 3);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               /* delay for cmd send */
>> +               usleep_range(300, 500);
>> +               /* wait for SPI ROM until not busy */
>> +               ret = ps8640_read(client, PAGE2_SWSPI_RDATA, &spi_status, 1);
>> +               if (ret)
>> +                       goto err_send_cmd_exit;
>> +
>> +               if (!(spi_status & BUSY))
>> +                       break;
>> +
>> +               if (ktime_compare(ktime_get(), timeout) > 0) {
>> +                       dev_err(&client->dev, "wait spi no busy timeout: %d\n",
>> +                               ret);
>> +                       return -EBUSY;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +
>> +err_send_cmd_exit:
>> +       dev_err(&client->dev, "send command err: %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_wait_rom_idle(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       int ret;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_wait_spi_ready(ps_bridge);
>> +       if (ret)
>> +               goto err_spi;
>> +
>> +       ret = ps8640_wait_spi_nobusy(ps_bridge);
>> +       if (ret)
>> +               goto err_spi;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       return 0;
>> +
>> +err_spi:
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +exit:
>> +       dev_err(&client->dev, "wait ps8640 rom idle fail: %d\n", ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int ps8640_spi_dl_mode(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       int ret;
>> +
>> +       /* switch ps8640 mode to spi dl mode */
>> +       if (ps_bridge->gpio_mode_sel)
>> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 0);
>> +
>> +       /* reset spi interface */
>> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET,
>> +                               SPI_SW_RESET | MPU_SW_RESET);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_write_byte(client, PAGE2_SW_RESET, MPU_SW_RESET);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       return 0;
>> +
>> +exit:
>> +       dev_err(&client->dev, "fail reset spi interface: %d\n", ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int ps8640_rom_prepare(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +       struct device *dev = &client->dev;
>> +       u8 i, cmd[2];
>> +       int ret;
>> +
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "failed enable-write-status-register: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       cmd[0] = WRITE_STATUS_REG_CMD;
>> +       cmd[1] = CLEAR_ALL_PROTECT;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> +       if (ret) {
>> +               dev_err(dev, "fail disable all protection: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* wait for SPI module ready */
>> +       ret = ps8640_wait_rom_idle(ps_bridge);
>> +       if (ret) {
>> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_ENABLE);
>> +       for (i = 0; i < ARRAY_SIZE(enc_ctrl_code); i++)
>> +               ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, enc_ctrl_code[i]);
>> +       ps8640_write_byte(client, PAGE2_IROM_CTRL, IROM_DISABLE);
>> +
>> +       /* Enable-Write-Status-Register */
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "fail enable-write-status-register: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* chip erase command */
>> +       cmd[0] = CHIP_ERASE_CMD;
>> +       ret = ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +       if (ret) {
>> +               dev_err(dev, "fail disable all protection: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ret = ps8640_wait_rom_idle(ps_bridge);
>> +       if (ret) {
>> +               dev_err(dev, "fail wait rom idle: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_check_chip_id(struct ps8640 *ps_bridge)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[4];
>> +       u8 buf[4];
>> +
>> +       ps8640_read(client, PAGE4_REV_L, buf, 4);
>> +       return memcmp(buf, hw_chip_id, sizeof(buf));
>> +}
>> +
>> +static int ps8640_validate_firmware(struct ps8640 *ps_bridge,
>> +                                   const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       u16 fw_chip_id;
>> +
>> +       /*
>> +        * Get the chip_id from the firmware. Make sure that it is the
>> +        * right controller to do the firmware and config update.
>> +        */
>> +       fw_chip_id = get_unaligned_le16(fw->data + FW_CHIP_ID_OFFSET);
>> +
>> +       if (fw_chip_id != 0x8640 && ps8640_check_chip_id(ps_bridge) == 0) {
>> +               dev_err(&client->dev,
>> +                       "chip id mismatch: fw 0x%x vs. chip 0x8640\n",
>> +                       fw_chip_id);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ps8640_write_rom(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       struct device *dev = &client->dev;
>> +       struct i2c_client *client2 = ps_bridge->page[2];
>> +       struct i2c_client *client7 = ps_bridge->page[7];
>> +       size_t pos, cpy_len;
>> +       u8 buf[257];
>> +       int ret;
>> +
>> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, I2C_TO_SPI_RESET);
>> +       msleep(100);
>> +       ps8640_write_byte(client2, PAGE2_SPI_CFG3, 0x00);
>> +
>> +       for (pos = 0; pos < fw->size; pos += cpy_len) {
>> +               buf[0] = PAGE2_ROMADD_BYTE1;
>> +               buf[1] = pos >> 8;
>> +               buf[2] = pos >> 16;
>> +               ret = ps8640_write_bytes(client2, buf, 3);
>> +               if (ret)
>> +                       goto error;
>> +               cpy_len = fw->size >= 256 + pos ? 256 : fw->size - pos;
>> +               buf[0] = 0;
>> +               memcpy(buf + 1, fw->data + pos, cpy_len);
>> +               ret = ps8640_write_bytes(client7, buf, cpy_len + 1);
>> +               if (ret)
>> +                       goto error;
>> +
>> +               dev_dbg(dev, "fw update completed %zu / %zu bytes\n", pos,
>> +                       fw->size);
>> +       }
>> +       return 0;
>> +
>> +error:
>> +       dev_err(dev, "failed write external flash, %d\n", ret);
>> +       return ret;
>> +}
>> +
>> +static int ps8640_spi_normal_mode(struct ps8640 *ps_bridge)
>> +{
>> +       u8 cmd[2];
>> +       struct i2c_client *client = ps_bridge->page[2];
>> +
>> +       /* Enable-Write-Status-Register */
>> +       cmd[0] = WRITE_ENABLE_CMD;
>> +       ps8640_spi_send_cmd(ps_bridge, cmd, 1);
>> +
>> +       /* protect BPL/BP0/BP1 */
>> +       cmd[0] = WRITE_STATUS_REG_CMD;
>> +       cmd[1] = BLK_PROTECT_BITS | STATUS_REG_PROTECT;
>> +       ps8640_spi_send_cmd(ps_bridge, cmd, 2);
>> +
>> +       /* wait for SPI rom ready */
>> +       ps8640_wait_rom_idle(ps_bridge);
>> +
>> +       /* disable PS8640 mapping function */
>> +       ps8640_write_byte(client, PAGE2_ENCTLSPI_WR, 0x00);
>> +
>> +       if (ps_bridge->gpio_mode_sel)
>> +               gpiod_set_value(ps_bridge->gpio_mode_sel, 1);
>> +       return 0;
>> +}
>> +
>> +static int ps8640_enter_bl(struct ps8640 *ps_bridge)
>> +{
>> +       ps_bridge->in_fw_update = true;
>> +       return ps8640_spi_dl_mode(ps_bridge);
>> +}
>> +
>> +static void ps8640_exit_bl(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       ps8640_spi_normal_mode(ps_bridge);
>> +       ps_bridge->in_fw_update = false;
>> +}
>> +
>> +static int ps8640_load_fw(struct ps8640 *ps_bridge, const struct firmware *fw)
>> +{
>> +       struct i2c_client *client = ps_bridge->page[0];
>> +       struct device *dev = &client->dev;
>> +       int ret;
>> +       bool ps8640_status_backup = ps_bridge->enabled;
>> +
>> +       ret = ps8640_validate_firmware(ps_bridge, fw);
>> +       if (ret)
>> +               return ret;
>> +
>> +       mutex_lock(&ps_bridge->fw_mutex);
>> +       if (!ps_bridge->in_fw_update) {
>> +               if (!ps8640_status_backup)
>> +                       ps8640_pre_enable(&ps_bridge->bridge);
>> +
>> +               ret = ps8640_enter_bl(ps_bridge);
>> +               if (ret)
>> +                       goto exit;
>> +       }
>> +
>> +       ret = ps8640_rom_prepare(ps_bridge);
>> +       if (ret)
>> +               goto exit;
>> +
>> +       ret = ps8640_write_rom(ps_bridge, fw);
>> +
>> +exit:
>> +       if (ret)
>> +               dev_err(dev, "Failed to load firmware, %d\n", ret);
>> +
>> +       ps8640_exit_bl(ps_bridge, fw);
>> +       if (!ps8640_status_backup)
>> +               ps8640_post_disable(&ps_bridge->bridge);
>> +       mutex_unlock(&ps_bridge->fw_mutex);
>> +       return ret;
>> +}
>> +
>> +static ssize_t ps8640_update_fw_store(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     const char *buf, size_t count)
>> +{
>> +       struct i2c_client *client = to_i2c_client(dev);
>> +       struct ps8640 *ps_bridge = i2c_get_clientdata(client);
>> +       const struct firmware *fw;
>> +       int error;
>> +
>> +       error = request_firmware(&fw, PS_FW_NAME, dev);
>> +       if (error) {
>> +               dev_err(dev, "Unable to open firmware %s: %d\n",
>> +                       PS_FW_NAME, error);
>> +               return error;
>> +       }
>> +
>> +       error = ps8640_load_fw(ps_bridge, fw);
>> +       if (error)
>> +               dev_err(dev, "The firmware update failed(%d)\n", error);
>> +       else
>> +               dev_info(dev, "The firmware update succeeded\n");
>> +
>> +       release_firmware(fw);
>> +       return error ? error : count;
>> +}
>> +
>> +static DEVICE_ATTR(fw_version, S_IRUGO, ps8640_fw_version_show, NULL);
>> +static DEVICE_ATTR(hw_version, S_IRUGO, ps8640_hw_version_show, NULL);
>> +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, ps8640_update_fw_store);
>> +
>> +static struct attribute *ps8640_attrs[] = {
>> +       &dev_attr_fw_version.attr,
>> +       &dev_attr_hw_version.attr,
>> +       &dev_attr_update_fw.attr,
>> +       NULL
>> +};
>> +
>> +static const struct attribute_group ps8640_attr_group = {
>> +       .attrs = ps8640_attrs,
>> +};
>> +
>> +static void ps8640_remove_sysfs_group(void *data)
>> +{
>> +       struct ps8640 *ps_bridge = data;
>> +
>> +       sysfs_remove_group(&ps_bridge->page[0]->dev.kobj, &ps8640_attr_group);
>> +}
>> +
>> +static int ps8640_probe(struct i2c_client *client,
>> +                       const struct i2c_device_id *id)
>> +{
>> +       struct device *dev = &client->dev;
>> +       struct ps8640 *ps_bridge;
>> +       struct device_node *np = dev->of_node;
>> +       struct device_node *port, *out_ep;
>> +       struct device_node *panel_node = NULL;
>> +       int ret;
>> +       u32 i;
>> +
>> +       ps_bridge = devm_kzalloc(dev, sizeof(*ps_bridge), GFP_KERNEL);
>> +       if (!ps_bridge)
>> +               return -ENOMEM;
>> +
>> +       /* port@1 is ps8640 output port */
>> +       port = of_graph_get_port_by_id(np, 1);
>> +       if (port) {
>> +               out_ep = of_get_child_by_name(port, "endpoint");
>> +               of_node_put(port);
>> +               if (out_ep) {
>> +                       panel_node = of_graph_get_remote_port_parent(out_ep);
>> +                       of_node_put(out_ep);
>> +               }
>> +       }
>> +       if (panel_node) {
>> +               ps_bridge->panel = of_drm_find_panel(panel_node);
>> +               of_node_put(panel_node);
>> +               if (!ps_bridge->panel)
>> +                       return -EPROBE_DEFER;
>> +       }
>> +
>> +       mutex_init(&ps_bridge->fw_mutex);
>> +       ps_bridge->supplies[0].supply = "vdd33";
>> +       ps_bridge->supplies[1].supply = "vdd12";
>> +       ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
>> +                                     ps_bridge->supplies);
>> +       if (ret) {
>> +               dev_info(dev, "failed to get regulators: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->gpio_mode_sel = devm_gpiod_get_optional(&client->dev,
>> +                                                            "mode-sel",
>> +                                                            GPIOD_OUT_HIGH);
>> +       if (IS_ERR(ps_bridge->gpio_mode_sel)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_mode_sel);
>> +               dev_err(dev, "cannot get mode-sel %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->gpio_power_down = devm_gpiod_get(&client->dev, "sleep",
>> +                                              GPIOD_OUT_LOW);
>> +       if (IS_ERR(ps_bridge->gpio_power_down)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_power_down);
>> +               dev_err(dev, "cannot get sleep: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /*
>> +        * Request the reset pin low to avoid the bridge being
>> +        * initialized prematurely
>> +        */
>> +       ps_bridge->gpio_reset = devm_gpiod_get(&client->dev, "reset",
>> +                                              GPIOD_OUT_LOW);
>> +       if (IS_ERR(ps_bridge->gpio_reset)) {
>> +               ret = PTR_ERR(ps_bridge->gpio_reset);
>> +               dev_err(dev, "cannot get reset: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       ps_bridge->bridge.funcs = &ps8640_bridge_funcs;
>> +       ps_bridge->bridge.of_node = dev->of_node;
>> +
>> +       ps_bridge->page[0] = client;
>> +       ps_bridge->ddc_i2c = i2c_new_dummy(client->adapter, EDID_I2C_ADDR);

I don't see why we need to create this dummy client. The drm edid helper
drm_get_edid() just needs the i2c adapter to which the client is connected.
It will internally initiate a read form the address EDID_I2C_ADDR.

I guess "drm_get_edid(connector, ps_bridge->page[0]->adapter)" should work.

>> +       if (!ps_bridge->ddc_i2c) {
>> +               dev_err(dev, "failed ddc_i2c dummy device, address%02x\n",
>> +                       EDID_I2C_ADDR);
>> +               return -EBUSY;
>> +       }
>> +       /*
>> +        * ps8640 uses multiple addresses, use dummy devices for them
>> +        * page[0]: for DP control
>> +        * page[1]: for VIDEO Bridge
>> +        * page[2]: for control top
>> +        * page[3]: for DSI Link Control1
>> +        * page[4]: for MIPI Phy
>> +        * page[5]: for VPLL
>> +        * page[6]: for DSI Link Control2

Does this chip support 2 DSI inputs, and we're just exposing one for now?
If so, we should probably revisit the DT bindings, so that port@2 doesn't
need to represent the 2nd DSI link.

Thanks,
Archit

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Jassi Brar @ 2016-11-11  5:45 UTC (permalink / raw)
  To: Horng-Shyang Liao
  Cc: Rob Herring, Matthias Brugger, Daniel Kurtz, Sascha Hauer,
	Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, linux-mediatek,
	srv_heupstream, Sascha Hauer, Philipp Zabel, Nicolas Boichat,
	CK HU, cawa cheng, Bibby Hsieh, YT Shen, Daoyuan Huang, Damon Chu
In-Reply-To: <1478776558.15447.2.camel@mtksdaap41>

On Thu, Nov 10, 2016 at 4:45 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
>> Hi,
>>
>> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
>> to help write registers with critical time limitation, such as
>> updating display configuration during the vblank. It controls Global
>> Command Engine (GCE) hardware to achieve this requirement.
>>
>> These patches have a build dependency on top of v4.9-rc1.
>>
>> Changes since v15:
>>  - separate "suspend and resume" patch from "save energy" patch
>>  - don't stop running tasks in cmdq_suspend()
>>    (i.e. leave no running tasks guarantee to clients)
>>
>> Best regards,
>> HS Liao
>>
>> HS Liao (5):
>>   dt-bindings: soc: Add documentation for the MediaTek GCE unit
>>   CMDQ: Mediatek CMDQ driver
>>   arm64: dts: mt8173: Add GCE node
>>   CMDQ: suspend and resume
>>   CMDQ: save energy
>>
>>  .../devicetree/bindings/mailbox/mtk-gce.txt        |  43 ++
>>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
>>  drivers/mailbox/Kconfig                            |  10 +
>>  drivers/mailbox/Makefile                           |   2 +
>>  drivers/mailbox/mtk-cmdq-mailbox.c                 | 632 +++++++++++++++++++++
>>  drivers/soc/mediatek/Kconfig                       |  11 +
>>  drivers/soc/mediatek/Makefile                      |   1 +
>>  drivers/soc/mediatek/mtk-cmdq-helper.c             | 310 ++++++++++
>>  include/linux/mailbox/mtk-cmdq-mailbox.h           |  67 +++
>>  include/linux/soc/mediatek/mtk-cmdq.h              | 182 ++++++
>>  10 files changed, 1268 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
>>  create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
>>  create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
>>  create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
>>  create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
>>
>
>
> Hi Jassi, Matthias,
>
> Sorry to disturb you.
>
No, you don't disturb, but the controller driver and protocol driver,
introduced in the same patch, does :)   So does the suspend/resume
support (patch 4&5) added  separately as a patch on top. Please
reorganise the patchset.

Thanks.

^ permalink raw reply

* Re: [RFC 5/5] doc_rst: media: New SDR formats SC16, SC18 & SC20
From: Antti Palosaari @ 2016-11-11  4:54 UTC (permalink / raw)
  To: Laurent Pinchart, Ramesh Shanmugasundaram
  Cc: robh+dt@kernel.org, mark.rutland@arm.com, mchehab@kernel.org,
	hverkuil@xs4all.nl, sakari.ailus@linux.intel.com, Chris Paterson,
	geert@linux-m68k.org, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <1805711.Tm0TqcXx1h@avalon>

Hello

On 11/10/2016 10:08 AM, Laurent Pinchart wrote:
> Antti, Hans, ping ? Please see below.
>
> On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
>>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
>>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
>>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
>>>>>>>
>>>>>>>> This patch adds documentation for the three new SDR formats
>>>>>>>>
>>>>>>>> V4L2_SDR_FMT_SCU16BE
>>>>>>>> V4L2_SDR_FMT_SCU18BE
>>>>>>>> V4L2_SDR_FMT_SCU20BE
>>>>>
>>>>> [snip]
>>>>>
>>>>>>>> +
>>>>>>>> +       -  start + 0:
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
>>>>>>>> +
>>>>>>>> +    -  .. row 2
>>>>>>>> +
>>>>>>>> +       -  start + buffer_size/2:
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
>>>>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
>>>>>>> formats.
>>>>>>
>>>>>> Thank you for bringing up this topic. This is one of the key design
>>>>>> dilemma.
>>>>>>
>>>>>> The I & Q data for these three SDR formats comes from two different
>>>>>> DMA channels and hence two separate pointers -> we could say it is
>>>>>> v4l2 multi- planar. Right now, I am making it look like a single
>>>>>> plane by presenting the data in one single buffer ptr.
>>>>>>
>>>>>> For e.g. multi-planar SC16 format would look something like this
>>>>>>
>>>>>> <------------------------32bits---------------------->
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
>>>>>> ...
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
>>>>>>
>>>>>> My concerns are
>>>>>>
>>>>>> 1) These formats are not a standard as the video "Image Formats".
>>>>>> These formats are possible when we use DRIF + MAX2175 combination.
>>>>>> If we interface with a different tuner vendor, the above format(s)
>>>>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
>>>>>> main open item for discussion in the cover letter.
>>>>
>>>> If the formats are really device-specific then they should be
>>>> documented accordingly and not made generic.
>>>>
>>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
>>>>>> correct me if this is wrong interpretation.
>>>>>>
>>>>>> - struct v4l2_format contains v4l2_sdr_format and
>>>>>> v4l2_pix_format_mplane as members of union. Should I create a new
>>>>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
>>>>>> of the video specific members would be unused (it would be similar
>>>>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
>>>>
>>>> I have no answer to that question as I'm not familiar with SDR. Antti,
>>>> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
>>>> as you've acked the patch, your input would be appreciated as well.
>>>
>>> If I understood correctly this hardware provides I and Q samples via
>>> different channels and driver now combines those channels as a sequential
>>> IQ sample pairs.
>>
>> The driver combines the two buffer ptrs and present as one single buffer.
>> For a buffer of size 200
>>
>> ptr + 0   : I I I I ... I
>> ptr + 100 : Q Q Q Q ... Q
>>
>>> I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
>>
>> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
>> Those modes are not added in the RFC patchset.
>>
>>> This is
>>> I I I I ... I
>>> Q Q Q Q ... Q
>>> I am not very familiar with planars, but it sounds like it is correct
>>> approach. So I think should be added rather than emulate packet
>>> sequential format.
>>
>> My understanding of V4L2 MPLANE constructs is limited to a quick code read
>> only. At this point MPLANE support seems specific to video. SDR is defined
>> as separate format like v4l2_pix_format. Questions would be - should we
>> define new SDR_MPLANE? or merge SDR format with pix format & reuse existing
>> MPLANE with some SDR extensions (if possible)? These seem big design
>> decisions. Any suggestions please?

struct v4l2_format contains union that has own format definition for 
video, video mplane and sdr (+many others). Basically on api there is 
own definitions for each type, so I think possible sdr mplane should be 
similarly own types and definitions.

>> For my use case, MPLANE support does not seem to add significant benefit
>> except it may be syntactically correct. I am doing cyclic DMA with a small
>> set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer
>> at two offsets. If we add MPLANE support, it can be two non-contiguous
>> buffer pointers.

If there is no clear idea about need of mplane then that's also fine for me.

And whole mplane concept is new for me. I have never played with any 
v4l2 video formats nor mplane video formats.

I would still like to hear what Hans think about adding mplane.

>>
>>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
>>>>>> propagated across the framework. Is this the preferred approach?
>>>>>>
>>>>>> It goes back to point (1). As of today, the change set for this
>>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
>>>>>> further SDR+MPLANE support to the framework as well?
>>>>>>
>>>>>> I would appreciate your suggestions on this regard.
>

regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply

* Re: [PATCH v2] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
From: Joel Stanley @ 2016-11-11  4:42 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Boris Brezillon, Marek Vasut, Richard Weinberger,
	Cyrille Pitchen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland
In-Reply-To: <1478688149-4554-1-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

On Wed, Nov 9, 2016 at 9:12 PM, Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:
> This driver adds mtd support for spi-nor attached to either or both of
> the Firmware Memory Controller or the SPI Flash Controller (AST2400
> only).
>
> The SMC controllers on the Aspeed AST2500 SoC are very similar to the
> ones found on the AST2400. The differences are on the number of
> supported flash modules and their default mappings in the SoC address
> space.
>
> The Aspeed AST2500 has one SPI controller for the BMC firmware and two
> for the host firmware. All controllers have now the same set of
> registers compatible with the AST2400 FMC controller and the legacy
> 'SMC' controller is fully gone.
>
> Each controller has a memory range on which it maps its flash module
> slaves. Each slave is assigned a memory window for its mapping that
> can be changed at bootime with the Segment Address Register.
>
> Each SPI flash slave can then be accessed in two modes: Command and
> User. When in User mode, accesses to the memory segment of the slaves
> are translated in SPI transfers. When in Command mode, the HW
> generates the SPI commands automatically and the memory segment is
> accessed as if doing a MMIO.
>
> Currently, only the User mode is supported. Command mode needs a
> little more work to check that the memory window on the AHB bus fits
> the module size.
>
> Based on previous work from Milton D. Miller II <miltonm-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
> ---
>  Tested on:
>
>  * OpenPOWER Palmetto (AST2400) with
>         FMC controller : n25q256a
>         SPI controller : mx25l25635e and n25q512ax3
>
>  * Evaluation board (AST2500) with
>         FMC controller : w25q256
>         SPI controller : w25q256
>
>  * OpenPOWER Witherspoon (AST2500) with
>         FMC controller : mx25l25635e * 2
>         SPI controller : mx66l1g45g

It's also in use on the Zaius and Barreleye OpenBMC systems.

In that respect, I would like to see this merged once it's deemed
ready by the mtd maintainers. We can follow up with the extra features
in future patches.

I've reviewed and tested this as Cedric and Milton have developed it,
so please add:

Reviewed-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

Cheers,

Joel

>
>  Changes since v2:
>
>  - added a set4b ops to handle difference in the controllers
>  - simplified the IO routines
>  - prepared for fast read using dummy cycles
>
>  Work in progress:
>
>  - read optimization using higher SPI clock frequencies
>  - command mode to direct reads from AHB
>  - DMA support
>
>  .../devicetree/bindings/mtd/aspeed-smc.txt         |  72 ++
>  drivers/mtd/spi-nor/Kconfig                        |  12 +
>  drivers/mtd/spi-nor/Makefile                       |   1 +
>  drivers/mtd/spi-nor/aspeed-smc.c                   | 783 +++++++++++++++++++++
>  4 files changed, 868 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/3] ARM: dts: imx6qdl-apalis: Do not rely on DDC I2C bus bitbang for HDMI
From: Stefan Agner @ 2016-11-11  4:27 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Shawn Guo
  Cc: Philipp Zabel, maitysanchayan, marcel.ziswiler, linux-arm-kernel,
	devicetree, linux-kernel
In-Reply-To: <7eb3df71-6d1a-006a-cc4c-83ff8df3c42c@mentor.com>

On 2016-11-10 10:29, Vladimir Zapolskiy wrote:
> Hi Stefan, Philipp,
> 
> On 11/09/2016 02:50 AM, Stefan Agner wrote:
>> On 2016-11-08 09:33, maitysanchayan@gmail.com wrote:
>>> Hello Shawn,
>>> 
>>> On 16-10-22 15:43:04, Vladimir Zapolskiy wrote:
>>>> Hi Shawn,
>>>> 
>>>> On 10/22/2016 06:25 AM, Shawn Guo wrote:
>>>>> On Mon, Sep 19, 2016 at 10:41:51AM +0530, Sanchayan Maity wrote:
>>>>>> Remove the use of DDC I2C bus bitbang to support reading of EDID
>>>>>> and rely on support from internal HDMI I2C master controller instead.
>>>>>> As a result remove the device tree property ddc-i2c-bus.
>>>>>> 
>>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>>>> 
>>>>> I think that the dw-hdmi i2c support [1] is a prerequisite of this
>>>>> patch.  I do not see it lands on v4.9-rc1.  Or am I missing something?
>>>>> 
>>>>> Shawn
>>>>> 
>>>>> [1] https://patchwork.kernel.org/patch/9296883/
>>>>> 
>>>> 
>>>> I'm adding Philipp to Cc, since he is the last one who tested the change
>>>> and helped me to push the change to the mainline:
>>>> 
>>>>   https://lists.freedesktop.org/archives/dri-devel/2016-September/118569.html
>>>> 
>>>> The problem is that there is no official DW HDMI bridge maintainer, may be
>>>> you can review the change, and if you find it satisfactory push it through
>>>> ARM/iMX tree.
>>> 
>>> Shawn, is it okay if that patch goes through your ARM/iMX tree?
>> 
>> I don't think it makes sense that the DRM bridge changes go through
>> Shawn's tree. Dave should merge Philipps pull request...
>> 
> 
> Philipp, do you mind to submit a rebased pull request one more time?
> 

You probably saw it, Dave merged it today in his drm-next branch:
https://cgit.freedesktop.org/~airlied/linux/?h=drm-next

I guess with that Shawn can go ahead and merge this patchset for next
too...?

--
Stefan

^ permalink raw reply

* Re: [PATCH v5 02/23] of: device: Export of_device_{get_modalias, uvent_modalias} to modules
From: Javier Martinez Canillas @ 2016-11-11  4:25 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: linux-usb, Arnd Bergmann, Neil Armstrong, Greg KH, Stephen Boyd,
	linux-kernel, Bjorn Andersson, devicetree, Peter Chen,
	linux-arm-kernel, linux-arm-msm, Andy Gross, Peter Chen,
	Felipe Balbi
In-Reply-To: <CAGb2v66H1doBOdPPN9kz1vdNYx8gpgkF6s3UhTiXCCp0RZYKwg@mail.gmail.com>

Hello ChenYu

On Fri, Nov 11, 2016 at 12:01 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Fri, Nov 11, 2016 at 5:42 AM, Rob Herring <robh@kernel.org> wrote:

[snip]

>>
>> 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.
>

Unfortunately the I2C core can't be changed without breaking a lot of
I2C drivers that are relying on the current behavior. I've already
posted a RFC patch [0] for I2C that does this about a year ago and
enumerated the issues that have to be addressed before the change can
be made (and fixed some of the issues mentioned) on this series [1].

Another issue is that an I2C device ID table is a requirement anyways
since I2C drivers expect an i2c_device_id as an argument of their
probe function. Kieran already have patches [2] to change that which
should land soon.

I plan to fix the remaining I2C drivers once his patches are merged
and re-post the RFC patch as a proper one.

> 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
>

Yes, this is the workaround used by most DT-only I2C drivers. The only
reason that these drivers have an I2C device ID is due the
restrictions imposed by the I2C core.

Best regards,
Javier

^ permalink raw reply

* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Jisheng Zhang @ 2016-11-11  3:33 UTC (permalink / raw)
  To: Ziji Hu, Rob Herring
  Cc: Gregory CLEMENT, Ulf Hansson, Adrian Hunter, linux-mmc,
	Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, devicetree,
	Thomas Petazzoni, linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu,
	Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
	Wei(SOCP) Liu, Wilson Ding, Xueping Liu, Hilbert
In-Reply-To: <20161111112243.7431625d@xhacker>

On Fri, 11 Nov 2016 11:22:43 +0800 Jisheng Zhang wrote:

> Hi Rob, Ziji,
> 
> On Thu, 10 Nov 2016 19:44:19 +0800 Ziji Hu wrote:
> 
> > Hi Rob,
> > 
> > On 2016/11/10 2:24, Rob Herring wrote:  
> > > On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:    
> > >> From: Ziji Hu <huziji@marvell.com>
> > >>
> > >> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> > >> Add Xenon-specific properties.
> > >> Also add properties for Xenon PHY setting.
> > >>
> > >> Signed-off-by: Hu Ziji <huziji@marvell.com>
> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > >> ---
> > >>  Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> > >>  MAINTAINERS                                                   |   1 +-
> > >>  2 files changed, 162 insertions(+), 0 deletions(-)
> > >>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> new file mode 100644
> > >> index 000000000000..0d2d139494d3
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> > >> @@ -0,0 +1,161 @@
> > >> +Marvell's Xenon SDHCI Controller device tree bindings
> > >> +This file documents differences between the core mmc properties
> > >> +described by mmc.txt and the properties used by the Xenon implementation.
> > >> +
> > >> +A single Xenon IP can support multiple slots.
> > >> +Each slot acts as an independent SDHC. It owns independent resources, such
> > >> +as register sets clock and PHY.
> > >> +Each slot should have an independent device tree node.
> > >> +
> > >> +Required Properties:
> > >> +- compatible: should be one of the following
> > >> +  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> > >> +  Must provide a second register area and marvell,pad-type.
> > >> +  - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> > >> +  Armada-3700.    
> > > 
> > > Need SoC specific compatible strings.
> > >     
> > 
> > 	Xenon SDHC is a common IP for all Marvell SOCs.
> > 	It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
> > 	There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
> > 	Actually only few SOCs require special properties.
> > 	Any suggestion please?
> >   
> > >> +
> > >> +- clocks:
> > >> +  Array of clocks required for SDHCI.
> > >> +  Requires at least one for Xenon IP core.
> > >> +  Some SOCs require additional clock for AXI bus.
> > >> +
> > >> +- clock-names:
> > >> +  Array of names corresponding to clocks property.
> > >> +  The input clock for Xenon IP core should be named as "core".
> > >> +  The optional AXI clock should be named as "axi".    
> > > 
> > > When is AXI clock optional? This should be required for ?? compatible 
> > > strings.
> > >     
> > 	It is required on some SOCs.
> > 	I will double check if a suitable compatible string can be determined for those SOCs.  
> 
> Besides the core clk, berlin SoCs have one AXI clock. Usually, we have two
> solutions:
> 
> solA: as current patch does, take "marvell,xenon-sdhci" as compatible string
> and make the AXI clock property optional. Usually for berlin SoCs, we don't need
> special properties.

Personally, I prefer solA: use the IP name as compatible string. This is IP
specific rather than SoC specific. The HW itself supports two clks

Thanks,
Jisheng

> 
> PS: this solution is also what sdhci-pxav3.c takes
> 
> solB: As Rob said, add extra SoC compatible strings, so we'll have
> something like:
> 
> static const struct of_device_id sdhci_xenon_of_match[] = {
> 	{ .compatible = "marvell,armada-3700-sdhci", },
> 	{ .compatible = "marvell,berlin4ct-sdhci", },
> 	...
> 	{ .compatible = "marvell,berlinxxx-mmc", },
> }
> 
> then we take care the AXI clk for berlin SoCs in the code.
> 
> 
> Which solution do you prefer?
> 
> Thanks,
> Jisheng


^ 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