Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] mmc: sdhci-of-arasan: Fix non static symbol warning
From: Wei Yongjun @ 2016-09-25 15:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Wei Yongjun <weiyongjun1@huawei.com>

Fixes the following sparse warning:

drivers/mmc/host/sdhci-of-arasan.c:253:6: warning:
 symbol 'sdhci_arasan_reset' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index da8e40a..e263671 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -250,7 +250,7 @@ static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
 	writel(vendor, host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
 }
 
-void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
+static void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
 {
 	u8 ctrl;
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

^ permalink raw reply related

* linux-next: manual merge of the arm-soc tree with Linus' tree
From: Robert Jarzmik @ 2016-09-25 16:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160905182603.GS1041@n2100.armlinux.org.uk>

Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Mon, Sep 05, 2016 at 10:58:03AM +1000, Stephen Rothwell wrote:
>> I fixed it up (I deleted the file) and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging.  You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.

...zip...
> Since the patch in my tree is fixing a regression caused by the broken
> conversions, I'm not mindful to drop it from the merge window - I
> actually want to push it into -rc kernels.  We _do_ need to fix the
> DT regressions, and quickly though, and push those with this patch.
>
> Since I don't use the SMC91x with DT, that's outside of what I can
> test, so consider this a call for help on that subject.
Then consider you have one more tester, for a 16-bits access.

My lubbock is now DT aware, with a smc91x ethernet card actually working,
provided the small patch in [2].

Cheers.

-- 
Robert

[1] DT extract
	ethernet at extbus {
		compatible = "smsc,lan91c94";
		reg = <0x0c000c00 0x100000 0x0e00000 0x100000>;
		reg-names = "smc91x-regs", "smc91x-attrib";
		interrupts-extended = <&pxa_cplds_irqs 3 IRQ_TYPE_EDGE_RISING>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_eth_default>;
		reg-io-width = <2>;
		reg-io-shift = <2>;
	};


[2] Patch for lubbock
---8>---
diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index 726b80f45906..6d4fdac6c0e0 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -2316,6 +2316,9 @@ static int smc_drv_probe(struct platform_device *pdev)
 		} else {
 			lp->cfg.flags |= SMC91X_USE_16BIT;
 		}
+		if (!device_property_read_u32(&pdev->dev, "reg-io-shift",
+					      &val))
+			lp->io_shift = val;
 	}
 #endif
 

^ permalink raw reply related

* [PATCH 5/5] arm64: Add uprobe support
From: Pratyush Anand @ 2016-09-25 17:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160923130528.GE2161@e104818-lin.cambridge.arm.com>

On Fri, Sep 23, 2016 at 6:35 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Fri, Sep 23, 2016 at 09:42:30AM +0530, Pratyush Anand wrote:
>> On 22/09/2016:05:50:30 PM, Catalin Marinas wrote:
>> > On Thu, Sep 22, 2016 at 08:53:28AM +0530, Pratyush Anand wrote:
>> > > On 21/09/2016:06:04:04 PM, Catalin Marinas wrote:

>> > As a quick workaround you could check mm->task_size > TASK_SIZE_32 in
>> > the arch_uprobe_analyze_insn() function.
>>
>> It would be doable. TASK_SIZE_32 is defined only for COMPAT. So, may be I can
>> return -EINVAL when mm->task_size < TASK_SIZE_64.
>
> That's just a temporary workaround. If we ever merge ILP32, this test
> would no longer be enough (as the ISA is AArch64 but with TASK_SIZE_32).

OK.. So what about doing something similar what x86 is doing.
We can have a flag for task Type in arch specific mm_context_t. We
also set this flag in COMPAT_SET_PERSONALITY() along with setting
thread_info flag, and we clear them in SET_PERSONALITY().

>
> Looking at prepare_uprobe(), we have a weak is_trap_insn() function.
> This check is meaningless without knowing which instruction set we
> target. A false positive here, however, is not that bad as we wouldn't
> end up inserting the wrong breakpoint in the executable. But it looks to
> me like the core uprobe code needs to pass some additional information
> like the type of task or ELF format to the arch code to make a useful
> choice of breakpoint type.

It seems that 'strtle r0, [r0], #160' would have the closest matching
aarch32 instruction wrt BRK64_OPCODE_UPROBES(0xd42000A0). But that too
seems a bad instruction. So, may be we can use still weak
is_trap_insn().

~Pratyush

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Sergio Prado @ 2016-09-25 17:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160917193123.762587d0@bbrezillon>

Hi Boris,

> > +Optional properties:
> > +- samsung,tacls : time for active CLE/ALE to nWE/nOE
> > +- samsung,twrph0 : active time for nWE/nOE
> > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive
> 
> Can you try to extract these information from the nand_sdr_timings
> struct instead of passing it in your DT?

I've tested and the nand chip I'm working on is not ONFI or JEDEC
compatible, so looks like it is not possible to extract the timing
information from nand_sdr_timings. Am I right?

> > +	samsung,ignore_unset_ecc;
> 
> Just discovered this ->ignore_unset_ecc property, and I don't
> understand why it's here for...
> Either you don't want to have ECC, and in this case you should set
> NAND_ECC_NONE, or you want to have ECC calculated, and in this case,
> the only valid situation where ECC bytes are 0xff is when the page is
> erased.
> 
> If I'm right, please fix the driver instead of adding this DT property.
> If I'm wrong, could you explain in more detail when you have ECC bytes
> set to 0xff?

I think you are right but I am not an expert on flash devices and the
MTD sub-system. The commit message of this code says "If a block's ecc
field is all 0xff, then ignore the ECC correction. This is for systems
where some of the blocks, such as the initial cramfs are written without
ECC and need to be loaded on start.". Does it make sense?

> > +	for_each_available_child_of_node(np, child) {
> > +
> > +		sets->name = (char *)child->name;
> > +		sets->of_node = child;
> > +		sets->nr_chips = 1;
> > +
> > +		if (!of_property_match_string(child, "nand-ecc-mode", "none"))
> > +			sets->disable_ecc = 1;
> > +
> > +		if (of_get_property(child, "nand-on-flash-bbt", NULL))
> > +			sets->flash_bbt = 1;
> > +
> 
> These properties are automatically extracted in nand_scan_ident(), why
> do you need to parse them twice?
> 

You are right, there is no need to parse them twice. But taking a look
at the code I found a problem. Right now enabling hardware ECC is done
at compile time by enabling CONFIG_MTD_NAND_S3C2410_HWECC in the
menuconfig. Looks like this config option should be removed so we can
select ECC mode using nand-ecc-mode property in the device tree. But
this would break current boards that are not using a device tree. So it
would be necessary to add a ecc_mode field in the platform_data of these
boards and set them all to the default ECC mode (NAND_ECC_SOFT). Is this
the right approach?

Thanks!

-- 
Sergio Prado
Embedded Labworks
Office: +55 11 2628-3461
Mobile: +55 11 97123-3420

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Boris Brezillon @ 2016-09-25 18:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160925174257.GA20238@sprado-desktop>

Hi Sergio,

On Sun, 25 Sep 2016 14:42:57 -0300
Sergio Prado <sergio.prado@e-labworks.com> wrote:

> Hi Boris,
> 
> > > +Optional properties:
> > > +- samsung,tacls : time for active CLE/ALE to nWE/nOE
> > > +- samsung,twrph0 : active time for nWE/nOE
> > > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive  
> > 
> > Can you try to extract these information from the nand_sdr_timings
> > struct instead of passing it in your DT?  
> 
> I've tested and the nand chip I'm working on is not ONFI or JEDEC
> compatible, so looks like it is not possible to extract the timing
> information from nand_sdr_timings. Am I right?

There's an default_onfi_timing_mode in the nand_flash_dev struct. You
can define a full-id for your NAND chip in the nand-ids table if you
want. Otherwise, this means you'll have to use ONFI timing mode 0
(which should work for all NANDs).

Note that we've recently introduced a generic interface [1] to let NAND
controllers configure the timings, and let the core select the best
timings based on the information it has (ONFI, JEDEC or the nand-ids
table).

> 
> > > +	samsung,ignore_unset_ecc;  
> > 
> > Just discovered this ->ignore_unset_ecc property, and I don't
> > understand why it's here for...
> > Either you don't want to have ECC, and in this case you should set
> > NAND_ECC_NONE, or you want to have ECC calculated, and in this case,
> > the only valid situation where ECC bytes are 0xff is when the page is
> > erased.
> > 
> > If I'm right, please fix the driver instead of adding this DT property.
> > If I'm wrong, could you explain in more detail when you have ECC bytes
> > set to 0xff?  
> 
> I think you are right but I am not an expert on flash devices and the
> MTD sub-system. The commit message of this code says "If a block's ecc
> field is all 0xff, then ignore the ECC correction. This is for systems
> where some of the blocks, such as the initial cramfs are written without
> ECC and need to be loaded on start.". Does it make sense?

Well, no, it does not make any sense. What could have a sense is
enabling ECC on some partitions, but not on others, but that's not
supported right now (actually, I tried to add support for that once,
but it was not accepted).

Do you really need that? I mean, is the platform you're trying to
convert to DT using this property in its platform_data objects?

I'd recommend to drop this property until we figure what it's really
used for.

> 
> > > +	for_each_available_child_of_node(np, child) {
> > > +
> > > +		sets->name = (char *)child->name;
> > > +		sets->of_node = child;
> > > +		sets->nr_chips = 1;
> > > +
> > > +		if (!of_property_match_string(child, "nand-ecc-mode", "none"))
> > > +			sets->disable_ecc = 1;
> > > +
> > > +		if (of_get_property(child, "nand-on-flash-bbt", NULL))
> > > +			sets->flash_bbt = 1;
> > > +  
> > 
> > These properties are automatically extracted in nand_scan_ident(), why
> > do you need to parse them twice?
> >   
> 
> You are right, there is no need to parse them twice. But taking a look
> at the code I found a problem. Right now enabling hardware ECC is done
> at compile time by enabling CONFIG_MTD_NAND_S3C2410_HWECC in the
> menuconfig. Looks like this config option should be removed so we can
> select ECC mode using nand-ecc-mode property in the device tree. But
> this would break current boards that are not using a device tree. So it
> would be necessary to add a ecc_mode field in the platform_data of these
> boards and set them all to the default ECC mode (NAND_ECC_SOFT). Is this
> the right approach?

It's the right approach, indeed.

Thanks,

Boris

[1]http://www.spinics.net/lists/arm-kernel/msg532007.html

^ permalink raw reply

* [PATCH 4/5] input: touchscreen: support Allwinner SoCs' touchscreen
From: Quentin Schulz @ 2016-09-25 19:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8174039f-777e-b4b8-58c9-3509748f2799@kernel.org>

On 24/07/2016 13:24, Jonathan Cameron wrote:
> On 20/07/16 09:29, Quentin Schulz wrote:
>> This adds support for Allwinner SoCs' (A10, A13 and A31) resistive
>> touchscreen. This driver is probed by the MFD sunxi-gpadc-mfd.
>>
>> This driver uses ADC channels exposed through the IIO framework by
>> sunxi-gpadc-iio to get its data. When opening this input device, it will
>> start buffering in the ADC driver and enable a TP_UP_PENDING irq. The ADC
>> driver will fill in a buffer with all data and call the callback the input
>> device associated with this buffer. The input device will then read the
>> buffer two by two and send X and Y coordinates to the input framework based
>> on what it received from the ADC's buffer. When closing this input device,
>> the buffering is stopped.
>>
>> Note that locations in the first received buffer after an TP_UP_PENDING irq
>> occurred are unreliable, thus dropped.
>>
> I think I now understand what you are doing.
> 
> The channel grab is grabbing 4 channels, when there are only two real
> ones (x and y) then you are abusing the callback interface from IIO.

Actually, I need an IIO channel only for registering the callback from
the consumer but I never use the IIO channel in an other way from
consumer side (everything's done in the provider by reading the FIFO
register and sending data to the callback via iio_buffer).

> That transmits only one scan (e.g. here (x,y)) per call. Because you
> have added a sideband route for the buffer size what you have wil work.
> 
> However, it is not how the interface should be used.  Please fix that.
> Using it correctly is not a big issue.
> 
> On the channels front, I'd be tempted to do this as follows:
> 
> 6 Channels, not 4.
> 
> First 4 are standard ADC channels
> Next 2 are the touch screen channels with appropriate descriptions
> (guessing these are actually differential channels across the various
> wires of the first two?)
> 

Yes they are differential channels.

However, I think there is actually no sense in using several channels
for the touchscreen as everything is handled by the hardware. You only
select the touchscreen mode by setting a register and then X and Y
coordinates will be added to the FIFO register when touching the
touchscreen. I would not mind not having IIO channel at all since we do
not read from the consumer. But I need a way to register a callback to
get data from the provider. I don't know if I make myself clear enough?

> Then you set the map up to apply to the last two channels only.
> By setting available_scan_masks to the relevant options in the IIO driver
> you'll be able to automatically lock the device when ever the
> touch screeen driver is loaded.
> 
> That map will be something like (in binary
> 000000
> 000011
> 000100
> 001000
> 010000
> 100000
> 001100
> 010100
> 011000
> 100100
> 101000
> 110000
> 011100
> 101100
> 110100
> 111000
> 111100
> 
> thus any combination of the ADC channels, but always all or none of the
> touchscreen (none when ever the ADC channels are on) the order above
> ensures we always turn on the minimum possible for a given requirement.
> 
> Hence whenever the touch screen is there it'll lock out the ADC usage.
> Your postenable can look at what is there and put it in the right mode.
> 

I'll dive more into that.

> After that, break your fifo read up into individual pairs of readings
> and push those out.
> 
> That way we end up using the interface in the standard fashion.
> 
> You'll also need fix the usage of the fifo for ADC mode which suffers
> from the same problem.  There all sorts of nasty crashes might occur
> or you might just loose data
> 
[...]
>> +/*
>> + * This function will be called by iio_push_to_buffers from another driver
>> + * (namely sunxi-gpadc-iio). It will be passed the buffer filled with input
>> + * values (X value then Y value) and the sunxi_gpadc_ts structure representing
>> + * the device.
>> + */
>> +static int sunxi_gpadc_ts_callback(const void *data, void *private)
>> +{
>> +	const struct sunxi_gpadc_buffer *buffer = data;
>> +	struct sunxi_gpadc_ts *info = private;
>> +	int i = 0;
>> +
>> +	/* Locations in the first buffer after an up event are unreliable */
>> +	if (info->ignore_fifo_data) {
>> +		info->ignore_fifo_data = false;
>> +		return 0;
>> +	}
>> +
> It doesn't work like this at all. You'll get one scan only on each call of
> this function.  As I said in the previous driver, if there is a true reason
> to do this (and I'm unconvinced as yet) then we need to add support in the
> iio core etc for this (and emulating it when multiple scan passing isn't
> happening).
> 
> I guess this will work, as you are passing the buffer size as a side
> band, but it is definitely not how that ABI is meant to be used.
> 
> Also, you grab 4 channels, and only two are used here.  Please explain...
> 

Hum. Actually, that's a mistake. I'm quiet confused about how I should
do it while I never read channels from the consumer. I still "need" one
for "linking" the consumer and the provider but in the meantime, I'm
never using that channel.

Thanks,
Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
From: Quentin Schulz @ 2016-09-25 19:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cbf9470e-62ee-0efe-d893-5e08df33576c@kernel.org>

On 25/09/2016 11:10, Jonathan Cameron wrote:
> On 24/09/16 18:40, Quentin Schulz wrote:
>> Hi Jonathan,
>>
>> Sorry for the (long) delay, I did not have time to work on it. I'll
>> mainly work in my free time now.
>>
>> Keep in mind this patch was proposed based on the v2 of the ADC patches.
>> Since then, substantial changes have been made and I'm working on
>> rebasing this series of patches on the v6, so comments here might
>> include references to code parts added later in the ADC patch series.
>>
>> On 24/07/2016 13:03, Jonathan Cameron wrote:
>>> On 20/07/16 09:29, Quentin Schulz wrote:
>>>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>>>> It also prepares the code which will be used by the touchscreen driver
>>>> named sunxi-gpadc-ts.
>>>>
>>>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>>>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>>>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>>>> consumer which will be in charge of reading data from these channels for
>>>> the input framework.
>>>>
>>>> The temperature can only be read when in touchscreen mode. This means if
>>>> the buffers are being used for the ADC, the temperature sensor cannot be
>>>> read.
>>> That may be the bizarest hardware restriction I've heard of in a while! :)
>>>>
>>>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>>>> and fill a buffer before sending it to the consumers which registered in
>>>> IIO for the ADC channels.
>>>>
>>>> When a consumer starts buffering ADC channels,
>>>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>>>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>>>> depending on a property of the DT ("allwinner,ts-attached").
>>>> When the consumer stops buffering, it disables the same irq.
>>> Hmm. Might be possible to distinguish which consumer caused the start.
>>> Thus, if the touchscreen is there we would know purely based on the
>>> driver being the requester that we need to be in touchscreen mode.
>>>
>>
>> As of yet, can't see in which way I can retrieve the consumer in
>> provider code. Maybe I'm missing something, I don't know?
> I don't think we have a current way of doing this... Might be possible
> to add one, but it would be a rather odd bit of reverse looking up.
[...]
>>>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>>>>  	unsigned int			fifo_data_irq;
>>>>  	unsigned int			temp_data_irq;
>>>>  	unsigned int			flags;
>>>> +	struct iio_dev			*indio_dev;
>>> I was suprised to see this as normally it is cleaner to structure
>>> the whole code to go in one direction through the structures (which is
>>> why we don't provide a generic iio_device_from_priv bit of pointer magic).
>>>
>>> Anyhow, don't htink you are actually using it ;)
>>>
>>
>> I'm using to push to buffers from the irq handler since I pass the local
>> structure (sunxi_gpadc_dev) to the irq handler when registering it. But
>> I guess I can pass the iio_dev instead and remove this from the local
>> structure.
> I'd prefer passing the iio_dev and keep all lookups in one direction.

ACK.

>>
>> [...]
>>>>  static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
>>>>  				int *val)
>>>>  {
>>>>  	struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>>>> +	bool buffered = info->buffered;
>>> Not worth the local version...
>>>>  	int ret = 0;
>>>> +	unsigned int reg;
>>>>  
>>>>  	mutex_lock(&indio_dev->mlock);
>>>>  
>>>>  	reinit_completion(&info->completion);
>>>> +
>>>> +	reg = SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) | SUNXI_GPADC_TP_FIFO_FLUSH;
>>>> +	regmap_update_bits(info->regmap, SUNXI_GPADC_TP_INT_FIFOC, reg, reg);
>>> I'd put it in directly rahter than having a reg local variable.  To mind
>>> mind that would be slightly easier to understand.
>>>> +
>>>>  	if (info->flags & SUNXI_GPADC_ARCH_SUN6I)
>>>>  		regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>>>>  			     SUNXI_GPADC_SUN6I_TP_MODE_EN |
>>>> @@ -153,9 +185,9 @@ static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
>>>>  			     SUNXI_GPADC_TP_MODE_EN |
>>>>  			     SUNXI_GPADC_TP_ADC_SELECT |
>>>>  			     SUNXI_GPADC_ADC_CHAN_SELECT(channel));
>>>> -	regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
>>>> -		     SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) |
>>>> -		     SUNXI_GPADC_TP_FIFO_FLUSH);
>>> Whole load of infrastructure in place to lock buffered mode out and
>>> revent transitions when we can't have them.
>>>
>>> iio_claim_direct_mode etc.  I think you can just use that here?
>>> If you need to do extra checks on it being enabled that should be
>>> fine too.
>>>
>>
>> Yes, way better with iio_device_claim_direct_mode and iio_buffer_enabled!
>>
>>> As a general rule, it makes sense to simply disable polled reads
>>> if in buffered mode.  Leads to much simpler code and generally
>>> the data is already known to userspace anyway.
>>>
>>
>> That's what I try to do.
>> However, I think the temperature of the SoC is an interesting feature to
>> have. Since it ("hardwarely") works while the ADC is read in touchscreen
>> mode (even in buffer mode), I guess it could be a good idea to allow it
>> in the driver. If we don't do that, boards with a touchscreen connected
>> to the ADC of the SoC will not get SoC temperatures and can't have
>> proper thermal management. We already have one board in that case: the
>> PocketCHIP.
>>
>> Therefore, I also need to know if when the buffer is enabled, if it's
>> for buffering ADC data or touchscreen data. If it's for ADC data, then I
>> should disable temperature readings since it will return senseless
>> values (from memory, always 0 which means something like -144?C).
> Nice so no thermal management if we don't have a touch screen :)

I think that's a big constraint. I think we should be able to read the
temperature when the ADC is in either touchscreen or ADC mode but ONLY
in DIRECT_MODE. While in BUFFER_MODE, we should enable temperature
reading only when the touchscreen is present.

That might induce a problem with the thermal framework I think. I've had
some problem with temperature reading's timeout so I'm reading a
manually "cached" temperature which is updated when the temperature
reading is done. However, it will never be updated when the ADC is in
buffered ADC mode. I don't know if in that case it is better to
unregister the thermal device, to give outdated values or to notify the
thermal framework the temperature readings timed out (which is verbose).
[...]
>>>> @@ -261,7 +302,29 @@ static irqreturn_t sunxi_gpadc_temp_data_irq_handler(int irq, void *dev_id)
>>>>  static irqreturn_t sunxi_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
>>>>  {
>>>>  	struct sunxi_gpadc_dev *info = dev_id;
>>>> -	int ret;
>>>> +	int ret, reg, i, fifo_count;
>>>> +
>>>> +	if (info->buffered) {
>>>> +		if (regmap_read(info->regmap, SUNXI_GPADC_TP_INT_FIFOS, &reg))
>>>> +			return IRQ_HANDLED;
>>>> +
>>>> +		fifo_count = (reg & SUNXI_GPADC_RXA_CNT) >> 8;
>>>> +		/* Sometimes, the interrupt occurs when the FIFO is empty. */
>>>> +		if (!fifo_count)
>>>> +			return IRQ_HANDLED;
>>>> +
>>>> +		for (i = 0; i < fifo_count; i++) {
>>>> +			if (regmap_read(info->regmap, SUNXI_GPADC_TP_DATA,
>>>> +					&info->buffer.buffer[i]))
>>>> +				return IRQ_HANDLED;
>>>> +		}
>>>> +
>>>> +		info->buffer.buff_size = i;
>>>> +
>>>> +		iio_push_to_buffers(info->indio_dev, &info->buffer);
>>> This is expecting a single 'scan' - e.g. set of channels read at one
>>> time.  Here I think we could have repeated sets of channels?
>>> (at least that would be what is normally meant by a fifo in such
>>> a device).
>>>
>>> If so you need to read 'whole' scans and push them one at a time.
>>> We don't yet have a bulk iio_push_to_buffers, though we can add
>>> one if it makes sense.  Care will be needed though as we'd need
>>> handle the case of different consumers either supporting or
>>> not supporting this new functionality.  Not particularly hard though
>>> if it is worth doing.
>>
>> I didn't know it was meant for only one scan. Then I need a bulk
>> iio_push_to_buffers.
> We've had a few cases where that would be handy recently. 
> The bit that makes it complex is if we are doing any demux of the channels
> to multiple consumers.  Could be done by falling back to separating
> the scan's out and pushing them one by one through the demux though.
> Not sure there is a better way to do it though...
>>
>> I have a rather big problem. The whole first FIFO at each touch is
>> unusable so I have to drop it. I can detect the beginning of a touch
>> when the TP_UP irq occurs, then I know the next full FIFO the consumer
>> receives by callback is to be dropped. If I use push_to_buffers to send
>> coordinates by coordinates, the consumer has no mean to know when the
>> second FIFO (the first to be valid) starts and can be used. Either we
>> can find a way to notify the consumer of the start of a new FIFO or I
>> have to use a bulk iio_push_to_buffers.
> Nasty indeed.
>>
>> The workaround would be to register the TP_UP irq in the provider (the
>> ADC driver) and do not send the first FIFO to the consumer. But then, we
>> need a way to know which consumer requests buffering to know when to
>> enable this irq and do all touchscreen-only logic (dropping first
>> frame). And I guess we don't have something like that yet. Or I could
>> only code a buffering in touchscreen mode and add the ADC buffering
>> later? But it doesn't feel right to do what I think should be handled
>> (TP_UP irq handler and first FIFO dropping) in the consumer, in the
>> provider.
> Would indeed by the nicer way of doing it, but we are ultimately working
> around a hardware issue (to my mind it should never return rubbish!)
> so I'd not worry too much about where the fix is.
>>
>> So it's quiet a dead-end yet if I can't use iio_push_to_buffers with a
>> whole FIFO (which you told is not how it is meant to be used).
> It only worked here because you had control of both ends of the link.
> 
> First thought is that we should add a bulk push to buffers, but
> that a little bit of fiddly code would be needed to unwind the
> data in the demux if needed.  Probably not too hard to do.  It would then
> need to repackage the data up as a bulk buffer data block to send onwards.
> 
> To do this I think you'd need to:
> 1) Add core support to have a bulk push with the right magic around to call
>    the demux code in a loop over all the elements before pushing on.
> 2) Bulk handling in the callback buffer.
> 3) Kfifo bulk handling (mostly to allow us to test the demux code).
> 
> Actually, short of stuff I haven't thought of, doesn't look too tricky
> and useful feature to have in general.
> 

Then I guess we'll have to do it :) I might need a lot of guiding though
but maybe that's more of an IRC chat or non-lkml conversation?

Thanks,
Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH v2 2/2] drm: zte: add initial vou drm driver
From: Daniel Vetter @ 2016-09-25 20:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474727185-24180-3-git-send-email-shawn.guo@linaro.org>

On Sat, Sep 24, 2016 at 10:26:25PM +0800, Shawn Guo wrote:
> It adds the initial ZTE VOU display controller DRM driver.  There are
> still some features to be added, like overlay plane, scaling, and more
> output devices support.  But it's already useful with dual CRTCs and
> HDMI monitor working.
> 
> It's been tested on Debian Jessie LXDE desktop with modesetting driver.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

I've done a very quick read-through, looks real pretty. A few comments
below and in-line.

For testing, have you tried to run i-g-t validation tests per our
documentation? See https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#validating-changes-with-igt

Cheers, Daniel

> ---
>  drivers/gpu/drm/Kconfig          |   2 +
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/zte/Kconfig      |   8 +
>  drivers/gpu/drm/zte/Makefile     |   8 +
>  drivers/gpu/drm/zte/zx_crtc.c    | 691 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/zte/zx_crtc.h    |  47 +++
>  drivers/gpu/drm/zte/zx_drm_drv.c | 258 +++++++++++++++
>  drivers/gpu/drm/zte/zx_drm_drv.h |  22 ++
>  drivers/gpu/drm/zte/zx_hdmi.c    | 540 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/zte/zx_plane.c   | 362 ++++++++++++++++++++
>  drivers/gpu/drm/zte/zx_plane.h   |  26 ++
>  11 files changed, 1965 insertions(+)
>  create mode 100644 drivers/gpu/drm/zte/Kconfig
>  create mode 100644 drivers/gpu/drm/zte/Makefile
>  create mode 100644 drivers/gpu/drm/zte/zx_crtc.c
>  create mode 100644 drivers/gpu/drm/zte/zx_crtc.h
>  create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.c
>  create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.h
>  create mode 100644 drivers/gpu/drm/zte/zx_hdmi.c
>  create mode 100644 drivers/gpu/drm/zte/zx_plane.c
>  create mode 100644 drivers/gpu/drm/zte/zx_plane.h

New entry in MAINTAINERS listening you (and probably dri-devel as the m-l)
is missing.

> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a22b1b..a91f8cecbe0f 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -223,6 +223,8 @@ source "drivers/gpu/drm/hisilicon/Kconfig"
>  
>  source "drivers/gpu/drm/mediatek/Kconfig"
>  
> +source "drivers/gpu/drm/zte/Kconfig"
> +
>  # Keep legacy drivers last
>  
>  menuconfig DRM_LEGACY
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 439d89b25ae0..fe461c94d266 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -85,3 +85,4 @@ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>  obj-$(CONFIG_DRM_ARCPGU)+= arc/
>  obj-y			+= hisilicon/
> +obj-$(CONFIG_DRM_ZTE)	+= zte/
> diff --git a/drivers/gpu/drm/zte/Kconfig b/drivers/gpu/drm/zte/Kconfig
> new file mode 100644
> index 000000000000..4065b2840f1c
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/Kconfig
> @@ -0,0 +1,8 @@
> +config DRM_ZTE
> +	tristate "DRM Support for ZTE SoCs"
> +	depends on DRM && ARCH_ZX
> +	select DRM_KMS_CMA_HELPER
> +	select DRM_KMS_FB_HELPER
> +	select DRM_KMS_HELPER
> +	help
> +	  Choose this option to enable DRM on ZTE ZX SoCs.
> diff --git a/drivers/gpu/drm/zte/Makefile b/drivers/gpu/drm/zte/Makefile
> new file mode 100644
> index 000000000000..b40968dc749f
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/Makefile
> @@ -0,0 +1,8 @@
> +zxdrm-y := \
> +	zx_drm_drv.o \
> +	zx_crtc.o \
> +	zx_plane.o \
> +	zx_hdmi.o
> +
> +obj-$(CONFIG_DRM_ZTE) += zxdrm.o
> +
> diff --git a/drivers/gpu/drm/zte/zx_crtc.c b/drivers/gpu/drm/zte/zx_crtc.c
> new file mode 100644
> index 000000000000..818bf9072573
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_crtc.c
> @@ -0,0 +1,691 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_plane_helper.h>
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/of_address.h>
> +#include <video/videomode.h>
> +
> +#include "zx_drm_drv.h"
> +#include "zx_crtc.h"
> +#include "zx_plane.h"
> +
> +/* OSD (GPC_GLOBAL) registers */
> +#define OSD_INT_STA			0x04
> +#define OSD_INT_CLRSTA			0x08
> +#define OSD_INT_MSK			0x0c
> +#define OSD_INT_AUX_UPT			BIT(14)
> +#define OSD_INT_MAIN_UPT		BIT(13)
> +#define OSD_INT_GL1_LBW			BIT(10)
> +#define OSD_INT_GL0_LBW			BIT(9)
> +#define OSD_INT_VL2_LBW			BIT(8)
> +#define OSD_INT_VL1_LBW			BIT(7)
> +#define OSD_INT_VL0_LBW			BIT(6)
> +#define OSD_INT_BUS_ERR			BIT(3)
> +#define OSD_INT_CFG_ERR			BIT(2)
> +#define OSD_INT_ERROR (\
> +	OSD_INT_GL1_LBW | OSD_INT_GL0_LBW | \
> +	OSD_INT_VL2_LBW | OSD_INT_VL1_LBW | OSD_INT_VL0_LBW | \
> +	OSD_INT_BUS_ERR | OSD_INT_CFG_ERR \
> +)
> +#define OSD_INT_ENABLE (OSD_INT_ERROR | OSD_INT_AUX_UPT | OSD_INT_MAIN_UPT)
> +#define OSD_CTRL0			0x10
> +#define OSD_CTRL0_GL0_EN		BIT(7)
> +#define OSD_CTRL0_GL0_SEL		BIT(6)
> +#define OSD_CTRL0_GL1_EN		BIT(5)
> +#define OSD_CTRL0_GL1_SEL		BIT(4)
> +#define OSD_RST_CLR			0x1c
> +#define RST_PER_FRAME			BIT(19)
> +
> +/* Main/Aux channel registers */
> +#define OSD_MAIN_CHN			0x470
> +#define OSD_AUX_CHN			0x4d0
> +#define CHN_CTRL0			0x00
> +#define CHN_ENABLE			BIT(0)
> +#define CHN_CTRL1			0x04
> +#define CHN_SCREEN_W_SHIFT		18
> +#define CHN_SCREEN_W_MASK		(0x1fff << CHN_SCREEN_W_SHIFT)
> +#define CHN_SCREEN_H_SHIFT		5
> +#define CHN_SCREEN_H_MASK		(0x1fff << CHN_SCREEN_H_SHIFT)
> +#define CHN_UPDATE			0x08
> +
> +/* TIMING_CTRL registers */
> +#define TIMING_TC_ENABLE		0x04
> +#define AUX_TC_EN			BIT(1)
> +#define MAIN_TC_EN			BIT(0)
> +#define FIR_MAIN_ACTIVE			0x08
> +#define FIR_AUX_ACTIVE			0x0c
> +#define FIR_MAIN_H_TIMING		0x10
> +#define FIR_MAIN_V_TIMING		0x14
> +#define FIR_AUX_H_TIMING		0x18
> +#define FIR_AUX_V_TIMING		0x1c
> +#define SYNC_WIDE_SHIFT			22
> +#define SYNC_WIDE_MASK			(0x3ff << SYNC_WIDE_SHIFT)
> +#define BACK_PORCH_SHIFT		11
> +#define BACK_PORCH_MASK			(0x7ff << BACK_PORCH_SHIFT)
> +#define FRONT_PORCH_SHIFT		0
> +#define FRONT_PORCH_MASK		(0x7ff << FRONT_PORCH_SHIFT)
> +#define TIMING_CTRL			0x20
> +#define AUX_POL_SHIFT			3
> +#define AUX_POL_MASK			(0x7 << AUX_POL_SHIFT)
> +#define MAIN_POL_SHIFT			0
> +#define MAIN_POL_MASK			(0x7 << MAIN_POL_SHIFT)
> +#define POL_DE_SHIFT			2
> +#define POL_VSYNC_SHIFT			1
> +#define POL_HSYNC_SHIFT			0
> +#define TIMING_INT_CTRL			0x24
> +#define TIMING_INT_STATE		0x28
> +#define TIMING_INT_AUX_FRAME		BIT(3)
> +#define TIMING_INT_MAIN_FRAME		BIT(1)
> +#define TIMING_INT_AUX_FRAME_SEL_VSW	(0x2 << 10)
> +#define TIMING_INT_MAIN_FRAME_SEL_VSW	(0x2 << 6)
> +#define TIMING_INT_ENABLE (\
> +	TIMING_INT_MAIN_FRAME_SEL_VSW | TIMING_INT_AUX_FRAME_SEL_VSW | \
> +	TIMING_INT_MAIN_FRAME | TIMING_INT_AUX_FRAME \
> +)
> +#define TIMING_MAIN_SHIFT		0x2c
> +#define TIMING_AUX_SHIFT		0x30
> +#define H_SHIFT_VAL			0x0048
> +#define TIMING_MAIN_PI_SHIFT		0x68
> +#define TIMING_AUX_PI_SHIFT		0x6c
> +#define H_PI_SHIFT_VAL			0x000f
> +
> +/* DTRC registers */
> +#define DTRC_F0_CTRL			0x2c
> +#define DTRC_F1_CTRL			0x5c
> +#define DTRC_DECOMPRESS_BYPASS		BIT(17)
> +#define DTRC_DETILE_CTRL		0x68
> +#define TILE2RASTESCAN_BYPASS_MODE	BIT(30)
> +#define DETILE_ARIDR_MODE_MASK		(0x3 << 0)
> +#define DETILE_ARID_ALL			0
> +#define DETILE_ARID_IN_ARIDR		1
> +#define DETILE_ARID_BYP_BUT_ARIDR	2
> +#define DETILE_ARID_IN_ARIDR2		3
> +#define DTRC_ARID			0x6c
> +#define DTRC_DEC2DDR_ARID		0x70
> +
> +/* VOU_CTRL registers */
> +#define VOU_INF_EN			0x00
> +#define VOU_INF_CH_SEL			0x04
> +#define VOU_INF_DATA_SEL		0x08
> +#define VOU_SOFT_RST			0x14
> +#define VOU_CLK_SEL			0x18
> +#define VOU_CLK_GL1_SEL			BIT(5)
> +#define VOU_CLK_GL0_SEL			BIT(4)
> +#define VOU_CLK_REQEN			0x20
> +#define VOU_CLK_EN			0x24
> +
> +/* OTFPPU_CTRL registers */
> +#define OTFPPU_RSZ_DATA_SOURCE		0x04
> +
> +#define GL_NUM				2
> +#define VL_NUM				3
> +
> +enum vou_chn_type {
> +	VOU_CHN_MAIN,
> +	VOU_CHN_AUX,
> +};
> +
> +struct zx_crtc {
> +	struct drm_crtc crtc;
> +	struct drm_plane *primary;
> +	struct device *dev;
> +	void __iomem *chnreg;
> +	enum vou_chn_type chn_type;
> +	struct clk *pixclk;
> +};
> +
> +#define to_zx_crtc(x)	container_of(crtc, struct zx_crtc, crtc)
> +
> +struct zx_vou_hw {
> +	struct device *dev;
> +	void __iomem *osd;
> +	void __iomem *timing;
> +	void __iomem *vouctl;
> +	void __iomem *otfppu;
> +	void __iomem *dtrc;
> +	struct clk *axi_clk;
> +	struct clk *ppu_clk;
> +	struct clk *main_clk;
> +	struct clk *aux_clk;
> +	struct zx_crtc *main_crtc;
> +	struct zx_crtc *aux_crtc;
> +};
> +
> +static inline bool is_main_crtc(struct drm_crtc *crtc)
> +{
> +	struct zx_crtc *zcrtc = to_zx_crtc(crtc);
> +
> +	return zcrtc->chn_type == VOU_CHN_MAIN;
> +}
> +
> +void vou_inf_enable(struct vou_inf *inf)
> +{
> +	struct drm_encoder *encoder = inf->encoder;
> +	struct drm_device *drm = encoder->dev;
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct zx_vou_hw *vou = priv->vou;
> +	bool is_main = is_main_crtc(encoder->crtc);
> +	u32 data_sel_shift = inf->id << 1;
> +	u32 val;
> +
> +	/* Select data format */
> +	val = readl(vou->vouctl + VOU_INF_DATA_SEL);
> +	val &= ~(0x3 << data_sel_shift);
> +	val |= inf->data_sel << data_sel_shift;
> +	writel(val, vou->vouctl + VOU_INF_DATA_SEL);
> +
> +	/* Select channel */
> +	val = readl(vou->vouctl + VOU_INF_CH_SEL);
> +	if (is_main)
> +		val &= ~(1 << inf->id);
> +	else
> +		val |= 1 << inf->id;
> +	writel(val, vou->vouctl + VOU_INF_CH_SEL);
> +
> +	/* Select interface clocks */
> +	val = readl(vou->vouctl + VOU_CLK_SEL);
> +	if (is_main)
> +		val &= ~inf->clocks_sel_bits;
> +	else
> +		val |= inf->clocks_sel_bits;
> +	writel(val, vou->vouctl + VOU_CLK_SEL);
> +
> +	/* Enable interface clocks */
> +	val = readl(vou->vouctl + VOU_CLK_EN);
> +	val |= inf->clocks_en_bits;
> +	writel(val, vou->vouctl + VOU_CLK_EN);
> +
> +	/* Enable the device */
> +	val = readl(vou->vouctl + VOU_INF_EN);
> +	val |= 1 << inf->id;
> +	writel(val, vou->vouctl + VOU_INF_EN);
> +}
> +
> +void vou_inf_disable(struct vou_inf *inf)
> +{
> +	struct drm_encoder *encoder = inf->encoder;
> +	struct drm_device *drm = encoder->dev;
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct zx_vou_hw *vou = priv->vou;
> +	u32 val;
> +
> +	/* Disable the device */
> +	val = readl(vou->vouctl + VOU_INF_EN);
> +	val &= ~(1 << inf->id);
> +	writel(val, vou->vouctl + VOU_INF_EN);
> +
> +	/* Disable interface clocks */
> +	val = readl(vou->vouctl + VOU_CLK_EN);
> +	val &= ~inf->clocks_en_bits;
> +	writel(val, vou->vouctl + VOU_CLK_EN);
> +}
> +
> +static inline void vou_chn_set_update(struct zx_crtc *zcrtc)
> +{
> +	writel(1, zcrtc->chnreg + CHN_UPDATE);
> +}
> +
> +static void zx_crtc_enable(struct drm_crtc *crtc)
> +{
> +	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> +	struct zx_crtc *zcrtc = to_zx_crtc(crtc);
> +	struct zx_vou_hw *vou = dev_get_drvdata(zcrtc->dev);
> +	bool is_main = is_main_crtc(crtc);
> +	struct videomode vm;
> +	u32 pol = 0;
> +	u32 val;
> +
> +	drm_display_mode_to_videomode(mode, &vm);
> +
> +	/* Set up timing parameters */
> +	val = (vm.vactive - 1) << 16;
> +	val |= (vm.hactive - 1) & 0xffff;
> +	writel(val, vou->timing + (is_main ? FIR_MAIN_ACTIVE : FIR_AUX_ACTIVE));
> +
> +	val = ((vm.hsync_len - 1) << SYNC_WIDE_SHIFT) & SYNC_WIDE_MASK;
> +	val |= ((vm.hback_porch - 1) << BACK_PORCH_SHIFT) & BACK_PORCH_MASK;
> +	val |= ((vm.hfront_porch - 1) << FRONT_PORCH_SHIFT) & FRONT_PORCH_MASK;
> +	writel(val, vou->timing + (is_main ? FIR_MAIN_H_TIMING :
> +					     FIR_AUX_H_TIMING));
> +
> +	val = ((vm.vsync_len - 1) << SYNC_WIDE_SHIFT) & SYNC_WIDE_MASK;
> +	val |= ((vm.vback_porch - 1) << BACK_PORCH_SHIFT) & BACK_PORCH_MASK;
> +	val |= ((vm.vfront_porch - 1) << FRONT_PORCH_SHIFT) & FRONT_PORCH_MASK;
> +	writel(val, vou->timing + (is_main ? FIR_MAIN_V_TIMING :
> +					     FIR_AUX_V_TIMING));
> +
> +	/* Set up polarities */
> +	if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW)
> +		pol |= 1 << POL_VSYNC_SHIFT;
> +	if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
> +		pol |= 1 << POL_HSYNC_SHIFT;
> +
> +	val = readl(vou->timing + TIMING_CTRL);
> +	val &= ~(is_main ? MAIN_POL_MASK : AUX_POL_MASK);
> +	val |= pol << (is_main ? MAIN_POL_SHIFT : AUX_POL_SHIFT);
> +	writel(val, vou->timing + TIMING_CTRL);
> +
> +	/* Setup SHIFT register by following what ZTE BSP does */
> +	writel(H_SHIFT_VAL, vou->timing + (is_main ? TIMING_MAIN_SHIFT :
> +						     TIMING_AUX_SHIFT));
> +	writel(H_PI_SHIFT_VAL, vou->timing + (is_main ? TIMING_MAIN_PI_SHIFT :
> +							TIMING_AUX_PI_SHIFT));
> +
> +	/* Enable TIMING_CTRL */
> +	val = readl(vou->timing + TIMING_TC_ENABLE);
> +	val |= is_main ? MAIN_TC_EN : AUX_TC_EN;
> +	writel(val, vou->timing + TIMING_TC_ENABLE);
> +
> +	/* Configure channel screen size */
> +	val = readl(zcrtc->chnreg + CHN_CTRL1);
> +	val &= ~(CHN_SCREEN_W_MASK | CHN_SCREEN_H_MASK);
> +	val |= (vm.hactive << CHN_SCREEN_W_SHIFT) & CHN_SCREEN_W_MASK;
> +	val |= (vm.vactive << CHN_SCREEN_H_SHIFT) & CHN_SCREEN_H_MASK;
> +	writel(val, zcrtc->chnreg + CHN_CTRL1);
> +
> +	/* Update channel */
> +	vou_chn_set_update(zcrtc);
> +
> +	/* Enable channel */
> +	val = readl(zcrtc->chnreg + CHN_CTRL0);
> +	val |= CHN_ENABLE;
> +	writel(val, zcrtc->chnreg + CHN_CTRL0);
> +
> +	/* Enable Graphic Layer */
> +	val = readl(vou->osd + OSD_CTRL0);
> +	val |= is_main ? OSD_CTRL0_GL0_EN : OSD_CTRL0_GL1_EN;
> +	writel(val, vou->osd + OSD_CTRL0);
> +
> +	drm_crtc_vblank_on(crtc);
> +
> +	/* Enable pixel clock */
> +	clk_set_rate(zcrtc->pixclk, mode->clock * 1000);
> +	clk_prepare_enable(zcrtc->pixclk);
> +}
> +
> +static void zx_crtc_disable(struct drm_crtc *crtc)
> +{
> +	struct zx_crtc *zcrtc = to_zx_crtc(crtc);
> +	struct zx_vou_hw *vou = dev_get_drvdata(zcrtc->dev);
> +	bool is_main = is_main_crtc(crtc);
> +	u32 val;
> +
> +	clk_disable_unprepare(zcrtc->pixclk);
> +
> +	drm_crtc_vblank_off(crtc);
> +
> +	/* Disable Graphic Layer */
> +	val = readl(vou->osd + OSD_CTRL0);
> +	val &= ~(is_main ? OSD_CTRL0_GL0_EN : OSD_CTRL0_GL1_EN);
> +	writel(val, vou->osd + OSD_CTRL0);
> +
> +	/* Disable channel */
> +	val = readl(zcrtc->chnreg + CHN_CTRL0);
> +	val &= ~CHN_ENABLE;
> +	writel(val, zcrtc->chnreg + CHN_CTRL0);
> +
> +	/* Disable TIMING_CTRL */
> +	val = readl(vou->timing + TIMING_TC_ENABLE);
> +	val &= ~(is_main ? MAIN_TC_EN : AUX_TC_EN);
> +	writel(val, vou->timing + TIMING_TC_ENABLE);
> +}
> +
> +static void zx_crtc_atomic_begin(struct drm_crtc *crtc,
> +				 struct drm_crtc_state *state)
> +{
> +	struct drm_pending_vblank_event *event = crtc->state->event;
> +
> +	if (event) {
> +		crtc->state->event = NULL;
> +
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		if (drm_crtc_vblank_get(crtc) == 0)
> +			drm_crtc_arm_vblank_event(crtc, event);
> +		else
> +			drm_crtc_send_vblank_event(crtc, event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +	}
> +}
> +
> +static const struct drm_crtc_helper_funcs zx_crtc_helper_funcs = {
> +	.enable = zx_crtc_enable,
> +	.disable = zx_crtc_disable,
> +	.atomic_begin = zx_crtc_atomic_begin,
> +};
> +
> +static const struct drm_crtc_funcs zx_crtc_funcs = {
> +	.destroy = drm_crtc_cleanup,
> +	.set_config = drm_atomic_helper_set_config,
> +	.page_flip = drm_atomic_helper_page_flip,
> +	.reset = drm_atomic_helper_crtc_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static struct zx_crtc *zx_crtc_init(struct drm_device *drm,
> +				    enum vou_chn_type chn_type)
> +{
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct zx_vou_hw *vou = priv->vou;
> +	struct device *dev = vou->dev;
> +	struct zx_layer_data data;
> +	struct zx_crtc *zcrtc;
> +	int ret;
> +
> +	zcrtc = devm_kzalloc(dev, sizeof(*zcrtc), GFP_KERNEL);
> +	if (!zcrtc)
> +		return ERR_PTR(-ENOMEM);
> +
> +	zcrtc->dev = dev;
> +	zcrtc->chn_type = chn_type;
> +
> +	if (chn_type == VOU_CHN_MAIN) {
> +		data.layer = vou->osd + 0x130;
> +		data.csc = vou->osd + 0x580;
> +		data.hbsc = vou->osd + 0x820;
> +		data.rsz = vou->otfppu + 0x600;
> +		zcrtc->chnreg = vou->osd + OSD_MAIN_CHN;
> +	} else {
> +		data.layer = vou->osd + 0x200;
> +		data.csc = vou->osd + 0x5d0;
> +		data.hbsc = vou->osd + 0x860;
> +		data.rsz = vou->otfppu + 0x800;
> +		zcrtc->chnreg = vou->osd + OSD_AUX_CHN;
> +	}
> +
> +	zcrtc->pixclk = devm_clk_get(dev, (chn_type == VOU_CHN_MAIN) ?
> +					  "main_wclk" : "aux_wclk");
> +	if (IS_ERR(zcrtc->pixclk))
> +		return ERR_PTR(PTR_ERR(zcrtc->pixclk));
> +
> +	zcrtc->primary = zx_plane_init(drm, dev, &data, DRM_PLANE_TYPE_PRIMARY);
> +	if (IS_ERR(zcrtc->primary))
> +		return ERR_PTR(PTR_ERR(zcrtc->primary));
> +
> +	ret = drm_crtc_init_with_planes(drm, &zcrtc->crtc, zcrtc->primary, NULL,
> +					&zx_crtc_funcs, NULL);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	drm_crtc_helper_add(&zcrtc->crtc, &zx_crtc_helper_funcs);
> +
> +	return zcrtc;
> +}
> +
> +int zx_crtc_enable_vblank(struct drm_device *drm, unsigned int pipe)
> +{
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct zx_vou_hw *vou = priv->vou;
> +	u32 intctl;
> +
> +	intctl = readl(vou->timing + TIMING_INT_CTRL);
> +	if (pipe == 0)
> +		intctl |= TIMING_INT_MAIN_FRAME;
> +	else
> +		intctl |= TIMING_INT_AUX_FRAME;
> +	writel(intctl, vou->timing + TIMING_INT_CTRL);
> +
> +	return 0;
> +}
> +
> +void zx_crtc_disable_vblank(struct drm_device *drm, unsigned int pipe)
> +{
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct zx_vou_hw *vou = priv->vou;
> +	u32 intctl;
> +
> +	intctl = readl(vou->timing + TIMING_INT_CTRL);
> +	if (pipe == 0)
> +		intctl &= ~TIMING_INT_MAIN_FRAME;
> +	else
> +		intctl &= ~TIMING_INT_AUX_FRAME;
> +	writel(intctl, vou->timing + TIMING_INT_CTRL);
> +}
> +
> +static irqreturn_t vou_irq_handler(int irq, void *dev_id)
> +{
> +	struct zx_vou_hw *vou = dev_id;
> +	u32 state;
> +
> +	/* Handle TIMING_CTRL frame interrupts */
> +	state = readl(vou->timing + TIMING_INT_STATE);
> +	writel(state, vou->timing + TIMING_INT_STATE);
> +
> +	if (state & TIMING_INT_MAIN_FRAME)
> +		drm_crtc_handle_vblank(&vou->main_crtc->crtc);
> +
> +	if (state & TIMING_INT_AUX_FRAME)
> +		drm_crtc_handle_vblank(&vou->aux_crtc->crtc);
> +
> +	/* Handle OSD interrupts */
> +	state = readl(vou->osd + OSD_INT_STA);
> +	writel(state, vou->osd + OSD_INT_CLRSTA);
> +
> +	if (state & OSD_INT_MAIN_UPT) {
> +		vou_chn_set_update(vou->main_crtc);
> +		zx_plane_set_update(vou->main_crtc->primary);
> +	}
> +
> +	if (state & OSD_INT_AUX_UPT) {
> +		vou_chn_set_update(vou->aux_crtc);
> +		zx_plane_set_update(vou->aux_crtc->primary);
> +	}
> +
> +	if (state & OSD_INT_ERROR)
> +		dev_err(vou->dev, "OSD ERROR: 0x%08x!\n", state);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void vou_dtrc_init(struct zx_vou_hw *vou)
> +{
> +	u32 val;
> +
> +	val = readl(vou->dtrc + DTRC_DETILE_CTRL);
> +	/* Clear bit for bypass by ID */
> +	val &= ~TILE2RASTESCAN_BYPASS_MODE;
> +	/* Select ARIDR mode */
> +	val &= ~DETILE_ARIDR_MODE_MASK;
> +	val |= DETILE_ARID_IN_ARIDR;
> +	writel(val, vou->dtrc + DTRC_DETILE_CTRL);
> +
> +	/* Bypass decompression for both frames */
> +	val = readl(vou->dtrc + DTRC_F0_CTRL);
> +	val |= DTRC_DECOMPRESS_BYPASS;
> +	writel(val, vou->dtrc + DTRC_F0_CTRL);
> +
> +	val = readl(vou->dtrc + DTRC_F1_CTRL);
> +	val |= DTRC_DECOMPRESS_BYPASS;
> +	writel(val, vou->dtrc + DTRC_F1_CTRL);
> +
> +	/* Set up ARID register */
> +	val = 0x0e;
> +	val |= 0x0f << 8;
> +	val |= 0x0e << 16;
> +	val |= 0x0f << 24;
> +	writel(val, vou->dtrc + DTRC_ARID);
> +
> +	/* Set up DEC2DDR_ARID register */
> +	val = 0x0e;
> +	val |= 0x0f << 8;
> +	writel(val, vou->dtrc + DTRC_DEC2DDR_ARID);
> +}
> +
> +static void vou_hw_init(struct zx_vou_hw *vou)
> +{
> +	u32 val;
> +
> +	/* Set GL0 to main channel and GL1 to aux channel */
> +	val = readl(vou->osd + OSD_CTRL0);
> +	val &= ~OSD_CTRL0_GL0_SEL;
> +	val |= OSD_CTRL0_GL1_SEL;
> +	writel(val, vou->osd + OSD_CTRL0);
> +
> +	/* Release reset for all VOU modules */
> +	writel(~0, vou->vouctl + VOU_SOFT_RST);
> +
> +	/* Select main clock for GL0 and aux clock for GL1 module */
> +	val = readl(vou->vouctl + VOU_CLK_SEL);
> +	val &= ~VOU_CLK_GL0_SEL;
> +	val |= VOU_CLK_GL1_SEL;
> +	writel(val, vou->vouctl + VOU_CLK_SEL);
> +
> +	/* Enable clock auto-gating for all VOU modules */
> +	writel(~0, vou->vouctl + VOU_CLK_REQEN);
> +
> +	/* Enable all VOU module clocks */
> +	writel(~0, vou->vouctl + VOU_CLK_EN);
> +
> +	/* Clear both OSD and TIMING_CTRL interrupt state */
> +	writel(~0, vou->osd + OSD_INT_CLRSTA);
> +	writel(~0, vou->timing + TIMING_INT_STATE);
> +
> +	/* Enable OSD and TIMING_CTRL interrrupts */
> +	writel(OSD_INT_ENABLE, vou->osd + OSD_INT_MSK);
> +	writel(TIMING_INT_ENABLE, vou->timing + TIMING_INT_CTRL);
> +
> +	/* Select GPC as input to gl/vl scaler as a sane default setting */
> +	writel(0x2a, vou->otfppu + OTFPPU_RSZ_DATA_SOURCE);
> +
> +	/*
> +	 * Needs to reset channel and layer logic per frame when frame starts
> +	 * to get VOU work properly.
> +	 */
> +	val = readl(vou->osd + OSD_RST_CLR);
> +	val |= RST_PER_FRAME;
> +	writel(val, vou->osd + OSD_RST_CLR);
> +
> +	vou_dtrc_init(vou);
> +}
> +
> +static int zx_crtc_bind(struct device *dev, struct device *master, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct drm_device *drm = data;
> +	struct zx_drm_private *priv = drm->dev_private;
> +	struct resource *res;
> +	struct zx_vou_hw *vou;
> +	int irq;
> +	int ret;
> +
> +	vou = devm_kzalloc(dev, sizeof(*vou), GFP_KERNEL);
> +	if (!vou)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "osd");
> +	vou->osd = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vou->osd))
> +		return PTR_ERR(vou->osd);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "timing_ctrl");
> +	vou->timing = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vou->timing))
> +		return PTR_ERR(vou->timing);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dtrc");
> +	vou->dtrc = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vou->dtrc))
> +		return PTR_ERR(vou->dtrc);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vou_ctrl");
> +	vou->vouctl = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vou->vouctl))
> +		return PTR_ERR(vou->vouctl);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otfppu");
> +	vou->otfppu = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vou->otfppu))
> +		return PTR_ERR(vou->otfppu);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	vou->axi_clk = devm_clk_get(dev, "aclk");
> +	if (IS_ERR(vou->axi_clk))
> +		return PTR_ERR(vou->axi_clk);
> +
> +	vou->ppu_clk = devm_clk_get(dev, "ppu_wclk");
> +	if (IS_ERR(vou->ppu_clk))
> +		return PTR_ERR(vou->ppu_clk);
> +
> +	clk_prepare_enable(vou->axi_clk);
> +	clk_prepare_enable(vou->ppu_clk);
> +
> +	vou->dev = dev;
> +	priv->vou = vou;
> +	dev_set_drvdata(dev, vou);
> +
> +	vou_hw_init(vou);
> +
> +	ret = devm_request_irq(dev, irq, vou_irq_handler, 0, "zx_vou", vou);
> +	if (ret < 0)
> +		return ret;
> +
> +	vou->main_crtc = zx_crtc_init(drm, VOU_CHN_MAIN);
> +	if (IS_ERR(vou->main_crtc))
> +		return PTR_ERR(vou->main_crtc);
> +
> +	vou->aux_crtc = zx_crtc_init(drm, VOU_CHN_AUX);
> +	if (IS_ERR(vou->aux_crtc))
> +		return PTR_ERR(vou->aux_crtc);
> +
> +	return 0;
> +}
> +
> +static void zx_crtc_unbind(struct device *dev, struct device *master,
> +			   void *data)
> +{
> +	struct zx_vou_hw *vou = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(vou->axi_clk);
> +	clk_disable_unprepare(vou->ppu_clk);
> +}
> +
> +static const struct component_ops zx_crtc_component_ops = {
> +	.bind = zx_crtc_bind,
> +	.unbind = zx_crtc_unbind,
> +};
> +
> +static int zx_crtc_probe(struct platform_device *pdev)
> +{
> +	return component_add(&pdev->dev, &zx_crtc_component_ops);
> +}
> +
> +static int zx_crtc_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &zx_crtc_component_ops);
> +	return 0;
> +}
> +
> +static const struct of_device_id zx_crtc_of_match[] = {
> +	{ .compatible = "zte,zx296718-dpc", },
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, zx_crtc_of_match);
> +
> +struct platform_driver zx_crtc_driver = {
> +	.probe = zx_crtc_probe,
> +	.remove = zx_crtc_remove,
> +	.driver	= {
> +		.name = "zx-crtc",
> +		.of_match_table	= zx_crtc_of_match,
> +	},
> +};
> diff --git a/drivers/gpu/drm/zte/zx_crtc.h b/drivers/gpu/drm/zte/zx_crtc.h
> new file mode 100644
> index 000000000000..f889208054ce
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_crtc.h
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __ZX_CRTC_H__
> +#define __ZX_CRTC_H__
> +
> +#define VOU_CRTC_MASK		0x3
> +
> +/* VOU output interfaces */
> +enum vou_inf_id {
> +	VOU_HDMI	= 0,
> +	VOU_RGB_LCD	= 1,
> +	VOU_TV_ENC	= 2,
> +	VOU_MIPI_DSI	= 3,
> +	VOU_LVDS	= 4,
> +	VOU_VGA		= 5,
> +};
> +
> +enum vou_inf_data_sel {
> +	VOU_YUV444	= 0,
> +	VOU_RGB_101010	= 1,
> +	VOU_RGB_888	= 2,
> +	VOU_RGB_666	= 3,
> +};
> +
> +struct vou_inf {
> +	struct drm_encoder *encoder;
> +	enum vou_inf_id id;
> +	enum vou_inf_data_sel data_sel;
> +	u32 clocks_en_bits;
> +	u32 clocks_sel_bits;
> +};
> +
> +void vou_inf_enable(struct vou_inf *inf);
> +void vou_inf_disable(struct vou_inf *inf);
> +
> +int zx_crtc_enable_vblank(struct drm_device *drm, unsigned int pipe);
> +void zx_crtc_disable_vblank(struct drm_device *drm, unsigned int pipe);
> +
> +#endif /* __ZX_CRTC_H__ */
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
> new file mode 100644
> index 000000000000..51fafb8e5f43
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> @@ -0,0 +1,258 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/list.h>
> +#include <linux/of_graph.h>
> +#include <linux/of_platform.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> +
> +#include "zx_drm_drv.h"
> +#include "zx_crtc.h"
> +
> +static void zx_drm_fb_output_poll_changed(struct drm_device *drm)
> +{
> +	struct zx_drm_private *priv = drm->dev_private;
> +
> +	drm_fbdev_cma_hotplug_event(priv->fbdev);
> +}
> +
> +static const struct drm_mode_config_funcs zx_drm_mode_config_funcs = {
> +	.fb_create = drm_fb_cma_create,
> +	.output_poll_changed = zx_drm_fb_output_poll_changed,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static void zx_drm_lastclose(struct drm_device *drm)
> +{
> +	struct zx_drm_private *priv = drm->dev_private;
> +
> +	drm_fbdev_cma_restore_mode(priv->fbdev);
> +}
> +
> +static const struct file_operations zx_drm_fops = {
> +	.owner = THIS_MODULE,
> +	.open = drm_open,
> +	.release = drm_release,
> +	.unlocked_ioctl = drm_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl = drm_compat_ioctl,
> +#endif
> +	.poll = drm_poll,
> +	.read = drm_read,
> +	.llseek = noop_llseek,
> +	.mmap = drm_gem_cma_mmap,
> +};
> +
> +static struct drm_driver zx_drm_driver = {
> +	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> +			   DRIVER_ATOMIC,
> +	.lastclose = zx_drm_lastclose,
> +	.get_vblank_counter = drm_vblank_no_hw_counter,
> +	.enable_vblank = zx_crtc_enable_vblank,
> +	.disable_vblank = zx_crtc_disable_vblank,
> +	.gem_free_object = drm_gem_cma_free_object,
> +	.gem_vm_ops = &drm_gem_cma_vm_ops,
> +	.dumb_create = drm_gem_cma_dumb_create,
> +	.dumb_map_offset = drm_gem_cma_dumb_map_offset,
> +	.dumb_destroy = drm_gem_dumb_destroy,
> +	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> +	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> +	.gem_prime_export = drm_gem_prime_export,
> +	.gem_prime_import = drm_gem_prime_import,
> +	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> +	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> +	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> +	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> +	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> +	.fops = &zx_drm_fops,
> +	.name = "zx-vou",
> +	.desc = "ZTE VOU Controller DRM",
> +	.date = "20160811",
> +	.major = 1,
> +	.minor = 0,
> +};
> +
> +static int zx_drm_bind(struct device *dev)
> +{
> +	struct drm_device *drm;
> +	struct zx_drm_private *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	drm = drm_dev_alloc(&zx_drm_driver, dev);
> +	if (!drm)
> +		return -ENOMEM;
> +
> +	drm->dev_private = priv;
> +	dev_set_drvdata(dev, drm);
> +
> +	drm_mode_config_init(drm);
> +	drm->mode_config.min_width = 16;
> +	drm->mode_config.min_height = 16;
> +	drm->mode_config.max_width = 4096;
> +	drm->mode_config.max_height = 4096;
> +	drm->mode_config.funcs = &zx_drm_mode_config_funcs;
> +
> +	ret = drm_dev_register(drm, 0);

drm_dev_register should be the last function call in your bind function.
Similar for unbind, drm_dev_register should be called first.

As a consequence of that you can remove the drm_connector_(un)register
calls, those are only needed for hotplugged connectors like dp mst. But
with correct ordering of drm_dev_(un)register that function will also take
care of connector registration and unregistration.

> +	if (ret)
> +		goto out_free;
> +
> +	ret = component_bind_all(dev, drm);
> +	if (ret) {
> +		DRM_ERROR("Failed to bind all components\n");
> +		goto out_unregister;
> +	}
> +
> +	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> +	if (ret < 0) {
> +		DRM_ERROR("failed to initialise vblank\n");
> +		goto out_unbind;
> +	}
> +
> +	/*
> +	 * We will manage irq handler on our own.  In this case, irq_enabled
> +	 * need to be true for using vblank core support.
> +	 */
> +	drm->irq_enabled = true;
> +
> +	drm_mode_config_reset(drm);
> +	drm_kms_helper_poll_init(drm);
> +
> +	priv->fbdev = drm_fbdev_cma_init(drm, 32, drm->mode_config.num_crtc,
> +					 drm->mode_config.num_connector);
> +	if (IS_ERR(priv->fbdev)) {
> +		ret = PTR_ERR(priv->fbdev);
> +		priv->fbdev = NULL;
> +		goto out_fini;
> +	}
> +
> +	return 0;
> +
> +out_fini:
> +	drm_kms_helper_poll_fini(drm);
> +	drm_mode_config_cleanup(drm);
> +	drm_vblank_cleanup(drm);
> +out_unbind:
> +	component_unbind_all(dev, drm);
> +out_unregister:
> +	drm_dev_unregister(drm);
> +out_free:
> +	dev_set_drvdata(dev, NULL);
> +	drm_dev_unref(drm);
> +	return ret;
> +}
> +
> +static void zx_drm_unbind(struct device *dev)
> +{
> +	struct drm_device *drm = dev_get_drvdata(dev);
> +	struct zx_drm_private *priv = drm->dev_private;
> +
> +	if (priv->fbdev) {
> +		drm_fbdev_cma_fini(priv->fbdev);
> +		priv->fbdev = NULL;
> +	}
> +	drm_kms_helper_poll_fini(drm);
> +	component_unbind_all(dev, drm);
> +	drm_vblank_cleanup(drm);
> +	drm_mode_config_cleanup(drm);
> +	drm_dev_unregister(drm);
> +	drm_dev_unref(drm);
> +	drm->dev_private = NULL;
> +	dev_set_drvdata(dev, NULL);
> +}
> +
> +static const struct component_master_ops zx_drm_master_ops = {
> +	.bind = zx_drm_bind,
> +	.unbind = zx_drm_unbind,
> +};
> +
> +static int compare_of(struct device *dev, void *data)
> +{
> +	return dev->of_node == data;
> +}
> +
> +static int zx_drm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *parent = dev->of_node;
> +	struct device_node *child;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = of_platform_populate(parent, NULL, NULL, dev);
> +	if (ret)
> +		return ret;
> +
> +	for_each_available_child_of_node(parent, child) {
> +		component_match_add(dev, &match, compare_of, child);
> +		of_node_put(child);
> +	}
> +
> +	return component_master_add_with_match(dev, &zx_drm_master_ops, match);
> +}
> +
> +static int zx_drm_remove(struct platform_device *pdev)
> +{
> +	component_master_del(&pdev->dev, &zx_drm_master_ops);
> +	return 0;
> +}
> +
> +static const struct of_device_id zx_drm_of_match[] = {
> +	{ .compatible = "zte,zx296718-vou", },
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, zx_drm_of_match);
> +
> +static struct platform_driver zx_drm_platform_driver = {
> +	.probe = zx_drm_probe,
> +	.remove = zx_drm_remove,
> +	.driver	= {
> +		.name = "zx-drm",
> +		.of_match_table	= zx_drm_of_match,
> +	},
> +};
> +
> +static struct platform_driver *drivers[] = {
> +	&zx_crtc_driver,
> +	&zx_hdmi_driver,
> +	&zx_drm_platform_driver,
> +};
> +
> +static int zx_drm_init(void)
> +{
> +	return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> +}
> +module_init(zx_drm_init);
> +
> +static void zx_drm_exit(void)
> +{
> +	platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
> +}
> +module_exit(zx_drm_exit);
> +
> +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> +MODULE_DESCRIPTION("ZTE ZX VOU DRM driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.h b/drivers/gpu/drm/zte/zx_drm_drv.h
> new file mode 100644
> index 000000000000..14c749949151
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __ZX_DRM_DRV_H__
> +#define __ZX_DRM_DRV_H__
> +
> +struct zx_drm_private {
> +	struct drm_fbdev_cma *fbdev;
> +	struct zx_vou_hw *vou;
> +};
> +
> +extern struct platform_driver zx_crtc_driver;
> +extern struct platform_driver zx_hdmi_driver;
> +
> +#endif /* __ZX_DRM_DRV_H__ */
> diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
> new file mode 100644
> index 000000000000..5aaab8493b1b
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_hdmi.c
> @@ -0,0 +1,540 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <drm/drm_of.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <linux/irq.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hdmi.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_device.h>
> +#include <linux/component.h>
> +
> +#include "zx_crtc.h"
> +
> +#define FUNC_SEL			0x000b
> +#define FUNC_HDMI_EN			BIT(0)
> +#define CLKPWD				0x000d
> +#define CLKPWD_PDIDCK			BIT(2)
> +#define PWD_SRST			0x0010
> +#define P2T_CTRL			0x0066
> +#define P2T_DC_PKT_EN			BIT(7)
> +#define L1_INTR_STAT			0x007e
> +#define L1_INTR_STAT_INTR1		BIT(0)
> +#define INTR1_STAT			0x008f
> +#define INTR1_MASK			0x0095
> +#define INTR1_MONITOR_DETECT		(BIT(5) | BIT(6))
> +#define ZX_DDC_ADDR			0x00ed
> +#define ZX_DDC_SEGM			0x00ee
> +#define ZX_DDC_OFFSET			0x00ef
> +#define ZX_DDC_DIN_CNT1			0x00f0
> +#define ZX_DDC_DIN_CNT2			0x00f1
> +#define ZX_DDC_CMD			0x00f3
> +#define DDC_CMD_MASK			0xf
> +#define DDC_CMD_CLEAR_FIFO		0x9
> +#define DDC_CMD_SEQUENTIAL_READ		0x2
> +#define ZX_DDC_DATA			0x00f4
> +#define ZX_DDC_DOUT_CNT			0x00f5
> +#define DDC_DOUT_CNT_MASK		0x1f
> +#define TEST_TXCTRL			0x00f7
> +#define TEST_TXCTRL_HDMI_MODE		BIT(1)
> +#define HDMICTL4			0x0235
> +#define TPI_HPD_RSEN			0x063b
> +#define TPI_HPD_CONNECTION		(BIT(1) | BIT(2))
> +#define TPI_INFO_FSEL			0x06bf
> +#define FSEL_AVI			0
> +#define FSEL_GBD			1
> +#define FSEL_AUDIO			2
> +#define FSEL_SPD			3
> +#define FSEL_MPEG			4
> +#define FSEL_VSIF			5
> +#define TPI_INFO_B0			0x06c0
> +#define TPI_INFO_EN			0x06df
> +#define TPI_INFO_TRANS_EN		BIT(7)
> +#define TPI_INFO_TRANS_RPT		BIT(6)
> +#define TPI_DDC_MASTER_EN		0x06f8
> +#define HW_DDC_MASTER			BIT(7)
> +
> +#define ZX_HDMI_INFOFRAME_SIZE		31
> +
> +struct zx_hdmi {
> +	struct drm_connector connector;
> +	struct drm_encoder encoder;
> +	struct device *dev;
> +	struct drm_device *drm;
> +	void __iomem *mmio;
> +	struct clk *cec_clk;
> +	struct clk *osc_clk;
> +	struct clk *xclk;
> +	bool sink_is_hdmi;
> +	bool sink_has_audio;
> +	struct vou_inf *inf;
> +};
> +
> +#define to_zx_hdmi(x)	container_of(x, struct zx_hdmi, x)
> +
> +static struct vou_inf vou_inf_hdmi = {
> +	.id = VOU_HDMI,
> +	.data_sel = VOU_YUV444,
> +	.clocks_en_bits = BIT(24) | BIT(18) | BIT(6),
> +	.clocks_sel_bits = BIT(13) | BIT(2),
> +};
> +
> +static inline u8 hdmi_readb(struct zx_hdmi *hdmi, u16 offset)
> +{
> +	return readl_relaxed(hdmi->mmio + offset * 4);
> +}
> +
> +static inline void hdmi_writeb(struct zx_hdmi *hdmi, u16 offset, u8 val)
> +{
> +	writel_relaxed(val, hdmi->mmio + offset * 4);
> +}
> +
> +static int zx_hdmi_infoframe_trans(struct zx_hdmi *hdmi,
> +				   union hdmi_infoframe *frame, u8 fsel)
> +{
> +	u8 buffer[ZX_HDMI_INFOFRAME_SIZE];
> +	u8 val;
> +	ssize_t num;
> +	int i;
> +
> +	hdmi_writeb(hdmi, TPI_INFO_FSEL, fsel);
> +
> +	num = hdmi_infoframe_pack(frame, buffer, ZX_HDMI_INFOFRAME_SIZE);
> +	if (num < 0)
> +		return num;
> +
> +	for (i = 0; i < num; i++)
> +		hdmi_writeb(hdmi, TPI_INFO_B0 + i, buffer[i]);
> +
> +	val = hdmi_readb(hdmi, TPI_INFO_EN);
> +	val |= TPI_INFO_TRANS_EN | TPI_INFO_TRANS_RPT;
> +	hdmi_writeb(hdmi, TPI_INFO_EN, val);
> +
> +	return num;
> +}
> +
> +static int zx_hdmi_config_video_vsi(struct zx_hdmi *hdmi,
> +				    struct drm_display_mode *mode)
> +{
> +	union hdmi_infoframe frame;
> +	int ret;
> +
> +	ret = drm_hdmi_vendor_infoframe_from_display_mode(&frame.vendor.hdmi,
> +							  mode);
> +	if (ret)
> +		return ret;
> +
> +	return zx_hdmi_infoframe_trans(hdmi, &frame, FSEL_VSIF);
> +}
> +
> +static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
> +				    struct drm_display_mode *mode)
> +{
> +	union hdmi_infoframe frame;
> +	int ret;
> +
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> +	if (ret)
> +		return ret;
> +
> +	/* We always use YUV444 for HDMI output. */
> +	frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> +
> +	return zx_hdmi_infoframe_trans(hdmi, &frame, FSEL_AVI);
> +}
> +
> +static void zx_hdmi_encoder_mode_set(struct drm_encoder *encoder,
> +				     struct drm_display_mode *mode,
> +				     struct drm_display_mode *adj_mode)
> +{
> +	struct zx_hdmi *hdmi = to_zx_hdmi(encoder);
> +
> +	if (hdmi->sink_is_hdmi) {
> +		zx_hdmi_config_video_avi(hdmi, mode);
> +		zx_hdmi_config_video_vsi(hdmi, mode);
> +	}
> +}
> +
> +static void zx_hdmi_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct zx_hdmi *hdmi = to_zx_hdmi(encoder);
> +
> +	vou_inf_enable(hdmi->inf);
> +}
> +
> +static void zx_hdmi_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct zx_hdmi *hdmi = to_zx_hdmi(encoder);
> +
> +	vou_inf_disable(hdmi->inf);
> +}
> +
> +static const struct drm_encoder_helper_funcs zx_hdmi_encoder_helper_funcs = {
> +	.enable	= zx_hdmi_encoder_enable,
> +	.disable = zx_hdmi_encoder_disable,
> +	.mode_set = zx_hdmi_encoder_mode_set,
> +};
> +
> +static const struct drm_encoder_funcs zx_hdmi_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static int zx_hdmi_get_edid_block(void *data, u8 *buf, unsigned int block,
> +				  size_t len)
> +{
> +	struct zx_hdmi *hdmi = data;
> +	int retry = 0;
> +	int ret = 0;
> +	int i = 0;
> +	u8 val;
> +
> +	/* Enable DDC master access */
> +	val = hdmi_readb(hdmi, TPI_DDC_MASTER_EN);
> +	val |= HW_DDC_MASTER;
> +	hdmi_writeb(hdmi, TPI_DDC_MASTER_EN, val);
> +
> +	hdmi_writeb(hdmi, ZX_DDC_ADDR, 0xa0);
> +	hdmi_writeb(hdmi, ZX_DDC_OFFSET, block * EDID_LENGTH);
> +	/* Bits [9:8] of bytes */
> +	hdmi_writeb(hdmi, ZX_DDC_DIN_CNT2, (len >> 8) & 0xff);
> +	/* Bits [7:0] of bytes */
> +	hdmi_writeb(hdmi, ZX_DDC_DIN_CNT1, len & 0xff);
> +
> +	/* Clear FIFO */
> +	val = hdmi_readb(hdmi, ZX_DDC_CMD);
> +	val &= ~DDC_CMD_MASK;
> +	val |= DDC_CMD_CLEAR_FIFO;
> +	hdmi_writeb(hdmi, ZX_DDC_CMD, val);
> +
> +	/* Kick off the read */
> +	val = hdmi_readb(hdmi, ZX_DDC_CMD);
> +	val &= ~DDC_CMD_MASK;
> +	val |= DDC_CMD_SEQUENTIAL_READ;
> +	hdmi_writeb(hdmi, ZX_DDC_CMD, val);

It looks like the ZX_DDC register range implements a hw i2c engine (since
you specifiy port and offsets and everything). Please implement it as an
i2c_adapter driver and use the normal drm_get_edid function.

> +
> +	while (len > 0) {
> +		int cnt, j;
> +
> +		/* FIFO needs some time to get ready */
> +		usleep_range(500, 1000);
> +
> +		cnt = hdmi_readb(hdmi, ZX_DDC_DOUT_CNT) & DDC_DOUT_CNT_MASK;
> +		if (cnt == 0) {
> +			if (++retry > 5) {
> +				dev_err(hdmi->dev, "DDC read timed out!");
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +			continue;
> +		}
> +
> +		for (j = 0; j < cnt; j++)
> +			buf[i++] = hdmi_readb(hdmi, ZX_DDC_DATA);
> +		len -= cnt;
> +	}
> +
> +	/* Disable DDC master access */
> +	val = hdmi_readb(hdmi, TPI_DDC_MASTER_EN);
> +	val &= ~HW_DDC_MASTER;
> +	hdmi_writeb(hdmi, TPI_DDC_MASTER_EN, val);
> +
> +	return ret;
> +}
> +
> +static int zx_hdmi_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct zx_hdmi *hdmi = to_zx_hdmi(connector);
> +	struct edid *edid;
> +	int ret = 0;
> +
> +	edid = drm_do_get_edid(connector, zx_hdmi_get_edid_block, hdmi);
> +	if (edid) {
> +		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
> +		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
> +		drm_mode_connector_update_edid_property(connector, edid);
> +		ret = drm_add_edid_modes(connector, edid);
> +		kfree(edid);
> +	}
> +
> +	return ret;
> +}
> +
> +static enum drm_mode_status
> +zx_hdmi_connector_mode_valid(struct drm_connector *connector,
> +			     struct drm_display_mode *mode)
> +{
> +	return MODE_OK;
> +}
> +
> +static struct drm_connector_helper_funcs zx_hdmi_connector_helper_funcs = {
> +	.get_modes = zx_hdmi_connector_get_modes,
> +	.mode_valid = zx_hdmi_connector_mode_valid,
> +};
> +
> +static enum drm_connector_status
> +zx_hdmi_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct zx_hdmi *hdmi = to_zx_hdmi(connector);
> +
> +	return (hdmi_readb(hdmi, TPI_HPD_RSEN) & TPI_HPD_CONNECTION) ?
> +		connector_status_connected : connector_status_disconnected;
> +}
> +
> +static void zx_hdmi_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_unregister(connector);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs zx_hdmi_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.detect = zx_hdmi_connector_detect,
> +	.destroy = zx_hdmi_connector_destroy,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int zx_hdmi_register(struct drm_device *drm, struct zx_hdmi *hdmi)
> +{
> +	struct drm_encoder *encoder = &hdmi->encoder;
> +
> +	encoder->possible_crtcs = VOU_CRTC_MASK;
> +
> +	drm_encoder_init(drm, encoder, &zx_hdmi_encoder_funcs,
> +			 DRM_MODE_ENCODER_TMDS, NULL);
> +	drm_encoder_helper_add(encoder, &zx_hdmi_encoder_helper_funcs);
> +
> +	hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +
> +	drm_connector_init(drm, &hdmi->connector, &zx_hdmi_connector_funcs,
> +			   DRM_MODE_CONNECTOR_HDMIA);
> +	drm_connector_helper_add(&hdmi->connector,
> +				 &zx_hdmi_connector_helper_funcs);
> +
> +	drm_mode_connector_attach_encoder(&hdmi->connector, encoder);
> +	drm_connector_register(&hdmi->connector);

As mentioned, these drm_connector_register/unregister calls aren't needed
with correct ordering in bind/unbind.

> +
> +	return 0;
> +}
> +
> +static irqreturn_t zx_hdmi_irq_thread(int irq, void *dev_id)
> +{
> +	struct zx_hdmi *hdmi = dev_id;
> +
> +	drm_helper_hpd_irq_event(hdmi->connector.dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t zx_hdmi_irq_handler(int irq, void *dev_id)
> +{
> +	struct zx_hdmi *hdmi = dev_id;
> +	u8 lstat;
> +
> +	lstat = hdmi_readb(hdmi, L1_INTR_STAT);
> +
> +	/* Monitor detect/HPD interrupt */
> +	if (lstat & L1_INTR_STAT_INTR1) {
> +		u8 stat = hdmi_readb(hdmi, INTR1_STAT);
> +
> +		hdmi_writeb(hdmi, INTR1_STAT, stat);
> +		if (stat & INTR1_MONITOR_DETECT)
> +			return IRQ_WAKE_THREAD;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static void zx_hdmi_phy_start(struct zx_hdmi *hdmi)
> +{
> +	/* Copy from ZTE BSP code */
> +	hdmi_writeb(hdmi, 0x222, 0x0);
> +	hdmi_writeb(hdmi, 0x224, 0x4);
> +	hdmi_writeb(hdmi, 0x909, 0x0);
> +	hdmi_writeb(hdmi, 0x7b0, 0x90);
> +	hdmi_writeb(hdmi, 0x7b1, 0x00);
> +	hdmi_writeb(hdmi, 0x7b2, 0xa7);
> +	hdmi_writeb(hdmi, 0x7b8, 0xaa);
> +	hdmi_writeb(hdmi, 0x7b2, 0xa7);
> +	hdmi_writeb(hdmi, 0x7b3, 0x0f);
> +	hdmi_writeb(hdmi, 0x7b4, 0x0f);
> +	hdmi_writeb(hdmi, 0x7b5, 0x55);
> +	hdmi_writeb(hdmi, 0x7b7, 0x03);
> +	hdmi_writeb(hdmi, 0x7b9, 0x12);
> +	hdmi_writeb(hdmi, 0x7ba, 0x32);
> +	hdmi_writeb(hdmi, 0x7bc, 0x68);
> +	hdmi_writeb(hdmi, 0x7be, 0x40);
> +	hdmi_writeb(hdmi, 0x7bf, 0x84);
> +	hdmi_writeb(hdmi, 0x7c1, 0x0f);
> +	hdmi_writeb(hdmi, 0x7c8, 0x02);
> +	hdmi_writeb(hdmi, 0x7c9, 0x03);
> +	hdmi_writeb(hdmi, 0x7ca, 0x40);
> +	hdmi_writeb(hdmi, 0x7dc, 0x31);
> +	hdmi_writeb(hdmi, 0x7e2, 0x04);
> +	hdmi_writeb(hdmi, 0x7e0, 0x06);
> +	hdmi_writeb(hdmi, 0x7cb, 0x68);
> +	hdmi_writeb(hdmi, 0x7f9, 0x02);
> +	hdmi_writeb(hdmi, 0x7b6, 0x02);
> +	hdmi_writeb(hdmi, 0x7f3, 0x0);
> +}
> +
> +static void zx_hdmi_hw_init(struct zx_hdmi *hdmi)
> +{
> +	u8 val;
> +
> +	/* Software reset */
> +	hdmi_writeb(hdmi, PWD_SRST, 1);
> +
> +	/* Enable pclk */
> +	val = hdmi_readb(hdmi, CLKPWD);
> +	val |= CLKPWD_PDIDCK;
> +	hdmi_writeb(hdmi, CLKPWD, val);
> +
> +	/* Enable HDMI for TX */
> +	val = hdmi_readb(hdmi, FUNC_SEL);
> +	val |= FUNC_HDMI_EN;
> +	hdmi_writeb(hdmi, FUNC_SEL, val);
> +
> +	/* Enable deep color packet */
> +	val = hdmi_readb(hdmi, P2T_CTRL);
> +	val |= P2T_DC_PKT_EN;
> +	hdmi_writeb(hdmi, P2T_CTRL, val);
> +
> +	/* Enable HDMI/MHL mode for output */
> +	val = hdmi_readb(hdmi, TEST_TXCTRL);
> +	val |= TEST_TXCTRL_HDMI_MODE;
> +	hdmi_writeb(hdmi, TEST_TXCTRL, val);
> +
> +	/* Configure reg_qc_sel */
> +	hdmi_writeb(hdmi, HDMICTL4, 0x3);
> +
> +	/* Enable interrupt */
> +	val = hdmi_readb(hdmi, INTR1_MASK);
> +	val |= INTR1_MONITOR_DETECT;
> +	hdmi_writeb(hdmi, INTR1_MASK, val);
> +
> +	/* Clear reset for normal operation */
> +	hdmi_writeb(hdmi, PWD_SRST, 0);
> +
> +	/* Start up phy */
> +	zx_hdmi_phy_start(hdmi);
> +}
> +
> +static int zx_hdmi_bind(struct device *dev, struct device *master, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct drm_device *drm = data;
> +	struct resource *res;
> +	struct zx_hdmi *hdmi;
> +	struct vou_inf *inf;
> +	int irq;
> +	int ret;
> +
> +	hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
> +	if (!hdmi)
> +		return -ENOMEM;
> +
> +	hdmi->dev = dev;
> +	hdmi->drm = drm;
> +
> +	inf = &vou_inf_hdmi;
> +	inf->encoder = &hdmi->encoder;
> +	hdmi->inf = inf;
> +
> +	dev_set_drvdata(dev, hdmi);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	hdmi->mmio = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(hdmi->mmio))
> +		return PTR_ERR(hdmi->mmio);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "osc_cec");
> +	if (IS_ERR(hdmi->cec_clk))
> +		return PTR_ERR(hdmi->cec_clk);
> +
> +	hdmi->osc_clk = devm_clk_get(hdmi->dev, "osc_clk");
> +	if (IS_ERR(hdmi->osc_clk))
> +		return PTR_ERR(hdmi->osc_clk);
> +
> +	hdmi->xclk = devm_clk_get(hdmi->dev, "xclk");
> +	if (IS_ERR(hdmi->xclk))
> +		return PTR_ERR(hdmi->xclk);
> +
> +	zx_hdmi_hw_init(hdmi);
> +
> +	clk_prepare_enable(hdmi->cec_clk);
> +	clk_prepare_enable(hdmi->osc_clk);
> +	clk_prepare_enable(hdmi->xclk);
> +
> +	ret = zx_hdmi_register(drm, hdmi);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_request_threaded_irq(dev, irq, zx_hdmi_irq_handler,
> +					zx_hdmi_irq_thread, IRQF_SHARED,
> +					dev_name(dev), hdmi);
> +
> +	return 0;
> +}
> +
> +static void zx_hdmi_unbind(struct device *dev, struct device *master,
> +			   void *data)
> +{
> +	struct zx_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(hdmi->cec_clk);
> +	clk_disable_unprepare(hdmi->osc_clk);
> +	clk_disable_unprepare(hdmi->xclk);
> +}
> +
> +static const struct component_ops zx_hdmi_component_ops = {
> +	.bind = zx_hdmi_bind,
> +	.unbind = zx_hdmi_unbind,
> +};
> +
> +static int zx_hdmi_probe(struct platform_device *pdev)
> +{
> +	return component_add(&pdev->dev, &zx_hdmi_component_ops);
> +}
> +
> +static int zx_hdmi_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &zx_hdmi_component_ops);
> +	return 0;
> +}
> +
> +static const struct of_device_id zx_hdmi_of_match[] = {
> +	{ .compatible = "zte,zx296718-hdmi", },
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, zx_hdmi_of_match);
> +
> +struct platform_driver zx_hdmi_driver = {
> +	.probe = zx_hdmi_probe,
> +	.remove = zx_hdmi_remove,
> +	.driver	= {
> +		.name = "zx-hdmi",
> +		.of_match_table	= zx_hdmi_of_match,
> +	},
> +};
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> new file mode 100644
> index 000000000000..326cc1ff7950
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -0,0 +1,362 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_plane_helper.h>
> +
> +#include "zx_crtc.h"
> +#include "zx_plane.h"
> +
> +/* GL registers */
> +#define GL_CTRL0			0x00
> +#define GL_UPDATE			BIT(5)
> +#define GL_CTRL1			0x04
> +#define GL_DATA_FMT_SHIFT		0
> +#define GL_DATA_FMT_MASK		(0xf << GL_DATA_FMT_SHIFT)
> +#define GL_FMT_ARGB8888			0
> +#define GL_FMT_RGB888			1
> +#define GL_FMT_RGB565			2
> +#define GL_FMT_ARGB1555			3
> +#define GL_FMT_ARGB4444			4
> +#define GL_CTRL2			0x08
> +#define GL_GLOBAL_ALPHA_SHIFT		8
> +#define GL_GLOBAL_ALPHA_MASK		(0xff << GL_GLOBAL_ALPHA_SHIFT)
> +#define GL_CTRL3			0x0c
> +#define GL_SCALER_BYPASS_MODE		BIT(0)
> +#define GL_STRIDE			0x18
> +#define GL_ADDR				0x1c
> +#define GL_SRC_SIZE			0x38
> +#define GL_SRC_W_SHIFT			16
> +#define GL_SRC_W_MASK			(0x3fff << GL_SRC_W_SHIFT)
> +#define GL_SRC_H_SHIFT			0
> +#define GL_SRC_H_MASK			(0x3fff << GL_SRC_H_SHIFT)
> +#define GL_POS_START			0x9c
> +#define GL_POS_END			0xa0
> +#define GL_POS_X_SHIFT			16
> +#define GL_POS_X_MASK			(0x1fff << GL_POS_X_SHIFT)
> +#define GL_POS_Y_SHIFT			0
> +#define GL_POS_Y_MASK			(0x1fff << GL_POS_Y_SHIFT)
> +
> +/* CSC registers */
> +#define CSC_CTRL0			0x30
> +#define CSC_COV_MODE_SHIFT		16
> +#define CSC_COV_MODE_MASK		(0xffff << CSC_COV_MODE_SHIFT)
> +#define CSC_BT601_IMAGE_RGB2YCBCR	0
> +#define CSC_BT601_IMAGE_YCBCR2RGB	1
> +#define CSC_BT601_VIDEO_RGB2YCBCR	2
> +#define CSC_BT601_VIDEO_YCBCR2RGB	3
> +#define CSC_BT709_IMAGE_RGB2YCBCR	4
> +#define CSC_BT709_IMAGE_YCBCR2RGB	5
> +#define CSC_BT709_VIDEO_RGB2YCBCR	6
> +#define CSC_BT709_VIDEO_YCBCR2RGB	7
> +#define CSC_BT2020_IMAGE_RGB2YCBCR	8
> +#define CSC_BT2020_IMAGE_YCBCR2RGB	9
> +#define CSC_BT2020_VIDEO_RGB2YCBCR	10
> +#define CSC_BT2020_VIDEO_YCBCR2RGB	11
> +#define CSC_WORK_ENABLE			BIT(0)
> +
> +/* RSZ registers */
> +#define RSZ_SRC_CFG			0x00
> +#define RSZ_DEST_CFG			0x04
> +#define RSZ_ENABLE_CFG			0x14
> +
> +/* HBSC registers */
> +#define HBSC_SATURATION			0x00
> +#define HBSC_HUE			0x04
> +#define HBSC_BRIGHT			0x08
> +#define HBSC_CONTRAST			0x0c
> +#define HBSC_THRESHOLD_COL1		0x10
> +#define HBSC_THRESHOLD_COL2		0x14
> +#define HBSC_THRESHOLD_COL3		0x18
> +#define HBSC_CTRL0			0x28
> +#define HBSC_CTRL_EN			BIT(2)
> +
> +struct zx_plane {
> +	struct drm_plane plane;
> +	void __iomem *layer;
> +	void __iomem *csc;
> +	void __iomem *hbsc;
> +	void __iomem *rsz;
> +};
> +
> +#define to_zx_plane(plane)	container_of(plane, struct zx_plane, plane)
> +
> +static const uint32_t gl_formats[] = {
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_RGB888,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_ARGB1555,
> +	DRM_FORMAT_ARGB4444,
> +};
> +
> +static int zx_gl_plane_atomic_check(struct drm_plane *plane,
> +				    struct drm_plane_state *state)
> +{
> +	u32 src_w, src_h;
> +
> +	src_w = state->src_w >> 16;
> +	src_h = state->src_h >> 16;
> +
> +	/* TODO: support scaling of the plane source */
> +	if ((src_w != state->crtc_w) || (src_h != state->crtc_h))
> +		return -EINVAL;

This is generally not enough checking. You probably need a call to
drm_plane_helper_check_state.

> +
> +	return 0;
> +}
> +
> +static int zx_gl_get_fmt(uint32_t format)
> +{
> +	switch (format) {
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_XRGB8888:
> +		return GL_FMT_ARGB8888;
> +	case DRM_FORMAT_RGB888:
> +		return GL_FMT_RGB888;
> +	case DRM_FORMAT_RGB565:
> +		return GL_FMT_RGB565;
> +	case DRM_FORMAT_ARGB1555:
> +		return GL_FMT_ARGB1555;
> +	case DRM_FORMAT_ARGB4444:
> +		return GL_FMT_ARGB4444;
> +	default:
> +		WARN_ONCE(1, "invalid pixel format %d\n", format);
> +		return -EINVAL;
> +	}
> +}
> +
> +static inline void zx_gl_set_update(struct zx_plane *zplane)
> +{
> +	void __iomem *layer = zplane->layer;
> +	u32 val;
> +
> +	val = readl(layer + GL_CTRL0);
> +	val |= GL_UPDATE;
> +	writel(val, layer + GL_CTRL0);
> +}
> +
> +static inline void zx_gl_rsz_set_update(struct zx_plane *zplane)
> +{
> +	writel(1, zplane->rsz + RSZ_ENABLE_CFG);
> +}
> +
> +void zx_plane_set_update(struct drm_plane *plane)
> +{
> +	struct zx_plane *zplane = to_zx_plane(plane);
> +
> +	zx_gl_rsz_set_update(zplane);
> +	zx_gl_set_update(zplane);
> +}
> +
> +static void zx_gl_rsz_setup(struct zx_plane *zplane, u32 src_w, u32 src_h,
> +			    u32 dst_w, u32 dst_h)
> +{
> +	void __iomem *rsz = zplane->rsz;
> +	u32 val;
> +
> +	val = ((src_h - 1) & 0xffff) << 16;
> +	val |= (src_w - 1) & 0xffff;
> +	writel(val, rsz + RSZ_SRC_CFG);
> +
> +	val = ((dst_h - 1) & 0xffff) << 16;
> +	val |= (dst_w - 1) & 0xffff;
> +	writel(val, rsz + RSZ_DEST_CFG);
> +
> +	zx_gl_rsz_set_update(zplane);
> +}
> +
> +static void zx_gl_plane_atomic_update(struct drm_plane *plane,
> +				      struct drm_plane_state *old_state)
> +{
> +	struct zx_plane *zplane = to_zx_plane(plane);
> +	struct drm_framebuffer *fb = plane->state->fb;
> +	struct drm_gem_cma_object *cma_obj;
> +	void __iomem *layer = zplane->layer;
> +	void __iomem *csc = zplane->csc;
> +	void __iomem *hbsc = zplane->hbsc;
> +	u32 src_x, src_y, src_w, src_h;
> +	u32 dst_x, dst_y, dst_w, dst_h;
> +	unsigned int depth, bpp;
> +	uint32_t format;
> +	dma_addr_t paddr;
> +	u32 stride;
> +	int fmt;
> +	u32 val;
> +
> +	if (!fb)
> +		return;
> +
> +	format = fb->pixel_format;
> +	stride = fb->pitches[0];
> +
> +	src_x = plane->state->src_x >> 16;
> +	src_y = plane->state->src_y >> 16;
> +	src_w = plane->state->src_w >> 16;
> +	src_h = plane->state->src_h >> 16;
> +
> +	dst_x = plane->state->crtc_x;
> +	dst_y = plane->state->crtc_y;
> +	dst_w = plane->state->crtc_w;
> +	dst_h = plane->state->crtc_h;
> +
> +	drm_fb_get_bpp_depth(format, &depth, &bpp);
> +
> +	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
> +	paddr = cma_obj->paddr + fb->offsets[0];
> +	paddr += src_y * stride + src_x * bpp / 8;
> +	writel(paddr, layer + GL_ADDR);
> +
> +	/* Set up source height/width register */
> +	val = (src_w << GL_SRC_W_SHIFT) & GL_SRC_W_MASK;
> +	val |= (src_h << GL_SRC_H_SHIFT) & GL_SRC_H_MASK;
> +	writel(val, layer + GL_SRC_SIZE);
> +
> +	/* Set up start position register */
> +	val = (dst_x << GL_POS_X_SHIFT) & GL_POS_X_MASK;
> +	val |= (dst_y << GL_POS_Y_SHIFT) & GL_POS_Y_MASK;
> +	writel(val, layer + GL_POS_START);
> +
> +	/* Set up end position register */
> +	val = ((dst_x + dst_w) << GL_POS_X_SHIFT) & GL_POS_X_MASK;
> +	val |= ((dst_y + dst_h) << GL_POS_Y_SHIFT) & GL_POS_Y_MASK;
> +	writel(val, layer + GL_POS_END);
> +
> +	/* Set up stride register */
> +	writel(stride & 0xffff, layer + GL_STRIDE);
> +
> +	/* Set up graphic layer data format */
> +	fmt = zx_gl_get_fmt(format);
> +	if (fmt >= 0) {
> +		val = readl(layer + GL_CTRL1);
> +		val &= ~GL_DATA_FMT_MASK;
> +		val |= fmt << GL_DATA_FMT_SHIFT;
> +		writel(val, layer + GL_CTRL1);
> +	}
> +
> +	/* Initialize global alpha with a sane value */
> +	val = readl(layer + GL_CTRL2);
> +	val &= ~GL_GLOBAL_ALPHA_MASK;
> +	val |= 0xff << GL_GLOBAL_ALPHA_SHIFT;
> +	writel(val, layer + GL_CTRL2);
> +
> +	/* Setup CSC for the GL */
> +	val = readl(csc + CSC_CTRL0);
> +	val &= ~CSC_COV_MODE_MASK;
> +	if (dst_h > 720)
> +		val |= CSC_BT709_IMAGE_RGB2YCBCR << CSC_COV_MODE_SHIFT;
> +	else
> +		val |= CSC_BT601_IMAGE_RGB2YCBCR << CSC_COV_MODE_SHIFT;
> +	val |= CSC_WORK_ENABLE;
> +	writel(val, csc + CSC_CTRL0);
> +
> +	/* Always use scaler since it exists */
> +	val = readl(layer + GL_CTRL3);
> +	val |= GL_SCALER_BYPASS_MODE;	/* set for not bypass */
> +	writel(val, layer + GL_CTRL3);
> +
> +	zx_gl_rsz_setup(zplane, src_w, src_h, dst_w, dst_h);
> +
> +	/* Enable HBSC block */
> +	val = readl(hbsc + HBSC_CTRL0);
> +	val |= HBSC_CTRL_EN;
> +	writel(val, hbsc + HBSC_CTRL0);
> +
> +	zx_gl_set_update(zplane);
> +}
> +
> +static const struct drm_plane_helper_funcs zx_gl_plane_helper_funcs = {
> +	.atomic_check = zx_gl_plane_atomic_check,
> +	.atomic_update = zx_gl_plane_atomic_update,
> +};
> +
> +static void zx_plane_destroy(struct drm_plane *plane)
> +{
> +	drm_plane_helper_disable(plane);
> +	drm_plane_cleanup(plane);
> +}
> +
> +static const struct drm_plane_funcs zx_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = zx_plane_destroy,
> +	.reset = drm_atomic_helper_plane_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +};
> +
> +static void zx_plane_hbsc_init(struct zx_plane *zplane)
> +{
> +	void __iomem *hbsc = zplane->hbsc;
> +
> +	/*
> +	 *  Initialize HBSC block with a sane configuration per recommedation
> +	 *  from ZTE BSP code.
> +	 */
> +	writel(0x200, hbsc + HBSC_SATURATION);
> +	writel(0x0, hbsc + HBSC_HUE);
> +	writel(0x0, hbsc + HBSC_BRIGHT);
> +	writel(0x200, hbsc + HBSC_CONTRAST);
> +
> +	writel((0x3ac << 16) | 0x40, hbsc + HBSC_THRESHOLD_COL1);
> +	writel((0x3c0 << 16) | 0x40, hbsc + HBSC_THRESHOLD_COL2);
> +	writel((0x3c0 << 16) | 0x40, hbsc + HBSC_THRESHOLD_COL3);
> +}
> +
> +struct drm_plane *zx_plane_init(struct drm_device *drm, struct device *dev,
> +				struct zx_layer_data *data,
> +				enum drm_plane_type type)
> +{
> +	const struct drm_plane_helper_funcs *helper;
> +	struct zx_plane *zplane;
> +	struct drm_plane *plane;
> +	const uint32_t *formats;
> +	unsigned int format_count;
> +	int ret;
> +
> +	zplane = devm_kzalloc(dev, sizeof(*zplane), GFP_KERNEL);
> +	if (!zplane)
> +		return ERR_PTR(-ENOMEM);
> +
> +	plane = &zplane->plane;
> +
> +	zplane->layer = data->layer;
> +	zplane->hbsc = data->hbsc;
> +	zplane->csc = data->csc;
> +	zplane->rsz = data->rsz;
> +
> +	zx_plane_hbsc_init(zplane);
> +
> +	switch (type) {
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		helper = &zx_gl_plane_helper_funcs;
> +		formats = gl_formats;
> +		format_count = ARRAY_SIZE(gl_formats);
> +		break;
> +	case DRM_PLANE_TYPE_OVERLAY:
> +		/* TODO: add video layer (vl) support */
> +		break;
> +	default:
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	ret = drm_universal_plane_init(drm, plane, VOU_CRTC_MASK,
> +				       &zx_plane_funcs, formats, format_count,
> +				       type, NULL);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	drm_plane_helper_add(plane, helper);
> +
> +	return plane;
> +}
> diff --git a/drivers/gpu/drm/zte/zx_plane.h b/drivers/gpu/drm/zte/zx_plane.h
> new file mode 100644
> index 000000000000..2b82cd558d9d
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_plane.h
> @@ -0,0 +1,26 @@
> +/*
> + * Copyright 2016 Linaro Ltd.
> + * Copyright 2016 ZTE Corporation.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __ZX_PLANE_H__
> +#define __ZX_PLANE_H__
> +
> +struct zx_layer_data {
> +	void __iomem *layer;
> +	void __iomem *csc;
> +	void __iomem *hbsc;
> +	void __iomem *rsz;
> +};
> +
> +struct drm_plane *zx_plane_init(struct drm_device *drm, struct device *dev,
> +				struct zx_layer_data *data,
> +				enum drm_plane_type type);
> +void zx_plane_set_update(struct drm_plane *plane);
> +
> +#endif /* __ZX_PLANE_H__ */
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* [PATCH] cpufreq: st: add missing \n to end of dev_err message
From: Colin King @ 2016-09-25 21:40 UTC (permalink / raw)
  To: linux-arm-kernel

From: Colin Ian King <colin.king@canonical.com>

Trival fix, dev_err message is missing a \n, so add it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/cpufreq/sti-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c
index 0404203..b366e6d 100644
--- a/drivers/cpufreq/sti-cpufreq.c
+++ b/drivers/cpufreq/sti-cpufreq.c
@@ -163,7 +163,7 @@ static int sti_cpufreq_set_opp_info(void)
 
 	reg_fields = sti_cpufreq_match();
 	if (!reg_fields) {
-		dev_err(dev, "This SoC doesn't support voltage scaling");
+		dev_err(dev, "This SoC doesn't support voltage scaling\n");
 		return -ENODEV;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/2] clk: rockchip: add I2S internal clock IDs for rk3288
From: Heiko Stuebner @ 2016-09-25 21:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160907191823.7f955ae2.john@metanate.com>

Hi John,

sorry this took so long,

Am Mittwoch, 7. September 2016, 19:18:23 CEST schrieb John Keeping:
> On Wed, 07 Sep 2016 19:58:31 +0200, Heiko Stuebner wrote:
> > Am Mittwoch, 7. September 2016, 17:53:29 CEST schrieb John Keeping:
> > > To minimize jitter on the I2S clocks, it is important that the
> > > denominator in the fractional divider is much greater than the
> > > numerator.  Add identifiers for these internal clocks so that the
> > > specific clock topology and rates can be specified in the device tree.
> > 
> > The TRM states that the denominator must be bigger than 20. Is this the
> > one
> > you found or did you find further constraints?
> 
> Is it not that the denominator must be bigger than 20 times the
> numerator?  That's what we found, although it seems that the greater the
> divisor the better, so our aim is dividing down from 594MHz from GPLL to
> the target rate in i2s_frac.

That is good to know ... the TRM probably lost the real information during 
translation or so and only states that hard value. Looking at the rk3368 TRM 
just now, the value really is 20 times greater than the numerator.


> > Did you try teaching the fractional divider to handle these constraints
> > before going this way?
> 
> No, I hadn't looked at doing that.  I'm not sure how that would work,
> we'd need to add support for fractional divider changing the parent rate
> wouldn't we?

Doug had a somewhat similar problem, mentioning the lack of a "bestdiv" 
equivalent that the standard divider provides. I'm not sure if there isn't a 
way to provide something like this and/or make the fractional clock honor 
specific requirments concerning numerator/denominator - any mathematicians 
around? :-)

One could also replace the generic fraction divider use with a rockchip-
specific implementation that somehow handles that.


> > Exporting the internal clocks really would be sort of plan d or e, after
> > handling this in the clock framework failed. Especially as i2s rates are
> > probably dependant on the media being handled (frequencies and such), so
> > setting fractional rates statically in the dts won't help you much in the
> > general case, as any new playback could trigger a clk_set_rate call
> > anyway?
> 
> We're not setting the fractional rate specifically, instead we're
> setting the i2s_pre rate and parent explicitly and banning the i2s_src
> mux setting to i2s_pre, which means that clk_set_rate on sclk_i2s0 will
> always change i2s_frac but leave i2s_pre alone.

But then you are again in the situation where some requested rate can ignore 
those fractional-divider constraints? In general I really don't like encoding 
such board-specific behaviour into the generic clock tree, like needing to 
disable parent relationships.

It works for your board now but does not necessarily for all other boards, 
which might require other "hacks" and also doesn't solve the general problem 
for all other fractional dividers we have.


Heiko

^ permalink raw reply

* [PATCH v2 0/4] ARM: imx31: clock initialization fixes
From: Vladimir Zapolskiy @ 2016-09-26  0:03 UTC (permalink / raw)
  To: linux-arm-kernel

The change is tested on qemu kzm target and mx31lite board, while both
targets don't have DTS in upstream, I had to write simple DTS files for
them, because the proposed change is for i.MX31 targets with OF support.

i.MX31/OF/clock initialization seems to be broken currently, if
the series is not applied I can not get a working clock source during
early boot stage on a board with DTB supplied.

Changes from v1 to v2, thanks to Uwe and Stephen for review:
* added one more new fix in imx31.dtsi which moves CCM device node
  to AIPS2 bus,
* included to the series a fix of CCM interrupts in imx31.dtsi,
  the change was sent as a separate patch, the change is included
  to avoid a patch application dependency,
* as suggested by Uwe reworded one of the commits removing "stack
  corruption" mentioning, the overwritten value is passed in a register,
* as suggested by Uwe squashed clk-imx31.c and imx31-dt.c changes
  to avoid a runtime problem if only one of two patches are applied

Vladimir Zapolskiy (4):
  ARM: dts: imx31: fix clock control module interrupts description
  ARM: dts: imx31: move CCM device node to AIPS2 bus devices
  clk: imx31: fix rewritten input argument of mx31_clocks_init()
  ARM: clk: imx31: properly init clocks for machines with DT

 .../devicetree/bindings/clock/imx31-clock.txt      |  2 +-
 arch/arm/boot/dts/imx31.dtsi                       | 14 +++---
 arch/arm/mach-imx/common.h                         |  1 -
 arch/arm/mach-imx/imx31-dt.c                       |  6 ---
 drivers/clk/imx/clk-imx31.c                        | 52 +++++++++++-----------
 5 files changed, 34 insertions(+), 41 deletions(-)

-- 
2.8.1

^ permalink raw reply

* [PATCH v2 1/4] ARM: dts: imx31: fix clock control module interrupts description
From: Vladimir Zapolskiy @ 2016-09-26  0:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474848223-19728-1-git-send-email-vz@mleia.com>

The type of AVIC interrupt controller found on i.MX31 is one-cell,
namely 31 for CCM DVFS and 53 for CCM, however for clock control
module its interrupts are specified as 3-cells, fix it.

Fixes: ef0e4a606fb6 ("ARM: mx31: Replace clk_register_clkdev with clock DT lookup")
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* added Rob's Acked-by tag.

 Documentation/devicetree/bindings/clock/imx31-clock.txt | 2 +-
 arch/arm/boot/dts/imx31.dtsi                            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/imx31-clock.txt b/Documentation/devicetree/bindings/clock/imx31-clock.txt
index 19df842..8163d56 100644
--- a/Documentation/devicetree/bindings/clock/imx31-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx31-clock.txt
@@ -77,7 +77,7 @@ Examples:
 clks: ccm at 53f80000{
 	compatible = "fsl,imx31-ccm";
 	reg = <0x53f80000 0x4000>;
-	interrupts = <0 31 0x04 0 53 0x04>;
+	interrupts = <31>, <53>;
 	#clock-cells = <1>;
 };
 
diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi
index 1ce7ae9..3e0893e 100644
--- a/arch/arm/boot/dts/imx31.dtsi
+++ b/arch/arm/boot/dts/imx31.dtsi
@@ -122,7 +122,7 @@
 			clks: ccm at 53f80000{
 				compatible = "fsl,imx31-ccm";
 				reg = <0x53f80000 0x4000>;
-				interrupts = <0 31 0x04 0 53 0x04>;
+				interrupts = <31>, <53>;
 				#clock-cells = <1>;
 			};
 		};
-- 
2.8.1

^ permalink raw reply related

* [PATCH v2 2/4] ARM: dts: imx31: move CCM device node to AIPS2 bus devices
From: Vladimir Zapolskiy @ 2016-09-26  0:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474848223-19728-1-git-send-email-vz@mleia.com>

i.MX31 Clock Control Module controller is found on AIPS2 bus, move it
there from SPBA bus to avoid a conflict of device IO space mismatch.

Fixes: ef0e4a606fb6 ("ARM: mx31: Replace clk_register_clkdev with clock DT lookup")
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* new change in the series

 arch/arm/boot/dts/imx31.dtsi | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi
index 3e0893e..8d4c0e3 100644
--- a/arch/arm/boot/dts/imx31.dtsi
+++ b/arch/arm/boot/dts/imx31.dtsi
@@ -118,13 +118,6 @@
 				interrupts = <19>;
 				clocks = <&clks 25>;
 			};
-
-			clks: ccm at 53f80000{
-				compatible = "fsl,imx31-ccm";
-				reg = <0x53f80000 0x4000>;
-				interrupts = <31>, <53>;
-				#clock-cells = <1>;
-			};
 		};
 
 		aips at 53f00000 { /* AIPS2 */
@@ -134,6 +127,13 @@
 			reg = <0x53f00000 0x100000>;
 			ranges;
 
+			clks: ccm at 53f80000{
+				compatible = "fsl,imx31-ccm";
+				reg = <0x53f80000 0x4000>;
+				interrupts = <31>, <53>;
+				#clock-cells = <1>;
+			};
+
 			gpt: timer at 53f90000 {
 				compatible = "fsl,imx31-gpt";
 				reg = <0x53f90000 0x4000>;
-- 
2.8.1

^ permalink raw reply related

* [PATCH v2 3/4] clk: imx31: fix rewritten input argument of mx31_clocks_init()
From: Vladimir Zapolskiy @ 2016-09-26  0:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474848223-19728-1-git-send-email-vz@mleia.com>

Function mx31_clocks_init() is called during clock intialization on
legacy boards with reference clock frequency passed as its input
argument, this can be verified by examination of the function
declaration found in arch/arm/mach-imx/common.h and actual function
users which include that header file.

Inside CCF driver the function ignores its input argument, by chance
the used value in the function body is the same as input arguments on
side of all callers.

Fixes: d9388c843237 ("clk: imx31: Do not call mxc_timer_init twice when booting with DT")
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* reworded commit message, thanks to Uwe for review

 drivers/clk/imx/clk-imx31.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/imx/clk-imx31.c b/drivers/clk/imx/clk-imx31.c
index 6a96414..6a49ba2 100644
--- a/drivers/clk/imx/clk-imx31.c
+++ b/drivers/clk/imx/clk-imx31.c
@@ -157,10 +157,8 @@ static void __init _mx31_clocks_init(unsigned long fref)
 	}
 }
 
-int __init mx31_clocks_init(void)
+int __init mx31_clocks_init(unsigned long fref)
 {
-	u32 fref = 26000000; /* default */
-
 	_mx31_clocks_init(fref);
 
 	clk_register_clkdev(clk[gpt_gate], "per", "imx-gpt.0");
-- 
2.8.1

^ permalink raw reply related

* [PATCH v2 4/4] ARM: clk: imx31: properly init clocks for machines with DT
From: Vladimir Zapolskiy @ 2016-09-26  0:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474848223-19728-1-git-send-email-vz@mleia.com>

Clock initialization for i.MX31 powered machines with DT support
should be done by a call of an init function registered with
CLK_OF_DECLARE() in common clock framework.

The change converts exported mx31_clocks_init_dt() into a static
initialization function registered by CLK_OF_DECLARE().

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* this change is a squashed change of arch/arm and clk changes,
  if only one of two changes is applied there is a compilation
  time or runtime issue.

 arch/arm/mach-imx/common.h   |  1 -
 arch/arm/mach-imx/imx31-dt.c |  6 ------
 drivers/clk/imx/clk-imx31.c  | 50 +++++++++++++++++++++++---------------------
 3 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index a8f4693..e97d8cb 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -48,7 +48,6 @@ int mx21_clocks_init(unsigned long lref, unsigned long fref);
 int mx27_clocks_init(unsigned long fref);
 int mx31_clocks_init(unsigned long fref);
 int mx35_clocks_init(void);
-int mx31_clocks_init_dt(void);
 struct platform_device *mxc_register_gpio(char *name, int id,
 	resource_size_t iobase, resource_size_t iosize, int irq, int irq_high);
 void mxc_set_cpu_type(unsigned int type);
diff --git a/arch/arm/mach-imx/imx31-dt.c b/arch/arm/mach-imx/imx31-dt.c
index 62e6b4f..668d74b 100644
--- a/arch/arm/mach-imx/imx31-dt.c
+++ b/arch/arm/mach-imx/imx31-dt.c
@@ -23,11 +23,6 @@ static const char * const imx31_dt_board_compat[] __initconst = {
 	NULL
 };
 
-static void __init imx31_dt_timer_init(void)
-{
-	mx31_clocks_init_dt();
-}
-
 /* FIXME: replace with DT binding */
 static const struct resource imx31_rnga_res[] __initconst = {
 	DEFINE_RES_MEM(MX31_RNGA_BASE_ADDR, SZ_16K),
@@ -43,7 +38,6 @@ DT_MACHINE_START(IMX31_DT, "Freescale i.MX31 (Device Tree Support)")
 	.map_io		= mx31_map_io,
 	.init_early	= imx31_init_early,
 	.init_irq	= mx31_init_irq,
-	.init_time	= imx31_dt_timer_init,
 	.init_machine	= imx31_dt_mach_init,
 	.dt_compat	= imx31_dt_board_compat,
 MACHINE_END
diff --git a/drivers/clk/imx/clk-imx31.c b/drivers/clk/imx/clk-imx31.c
index 6a49ba2..cbce308 100644
--- a/drivers/clk/imx/clk-imx31.c
+++ b/drivers/clk/imx/clk-imx31.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <soc/imx/revision.h>
 #include <soc/imx/timer.h>
 #include <asm/irq.h>
@@ -72,14 +73,8 @@ static struct clk ** const uart_clks[] __initconst = {
 	NULL
 };
 
-static void __init _mx31_clocks_init(unsigned long fref)
+static void __init _mx31_clocks_init(void __iomem *base, unsigned long fref)
 {
-	void __iomem *base;
-	struct device_node *np;
-
-	base = ioremap(MX31_CCM_BASE_ADDR, SZ_4K);
-	BUG_ON(!base);
-
 	clk[dummy] = imx_clk_fixed("dummy", 0);
 	clk[ckih] = imx_clk_fixed("ckih", fref);
 	clk[ckil] = imx_clk_fixed("ckil", 32768);
@@ -147,19 +142,17 @@ static void __init _mx31_clocks_init(unsigned long fref)
 	clk_prepare_enable(clk[iim_gate]);
 	mx31_revision();
 	clk_disable_unprepare(clk[iim_gate]);
-
-	np = of_find_compatible_node(NULL, NULL, "fsl,imx31-ccm");
-
-	if (np) {
-		clk_data.clks = clk;
-		clk_data.clk_num = ARRAY_SIZE(clk);
-		of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
-	}
 }
 
 int __init mx31_clocks_init(unsigned long fref)
 {
-	_mx31_clocks_init(fref);
+	void __iomem *base;
+
+	base = ioremap(MX31_CCM_BASE_ADDR, SZ_4K);
+	if (!base)
+		panic("%s: failed to map registers\n", __func__);
+
+	_mx31_clocks_init(base, fref);
 
 	clk_register_clkdev(clk[gpt_gate], "per", "imx-gpt.0");
 	clk_register_clkdev(clk[ipg], "ipg", "imx-gpt.0");
@@ -222,22 +215,31 @@ int __init mx31_clocks_init(unsigned long fref)
 	return 0;
 }
 
-int __init mx31_clocks_init_dt(void)
+static void __init mx31_clocks_init_dt(struct device_node *np)
 {
-	struct device_node *np;
+	struct device_node *osc_np;
 	u32 fref = 26000000; /* default */
+	void __iomem *ccm;
 
-	for_each_compatible_node(np, NULL, "fixed-clock") {
-		if (!of_device_is_compatible(np, "fsl,imx-osc26m"))
+	for_each_compatible_node(osc_np, NULL, "fixed-clock") {
+		if (!of_device_is_compatible(osc_np, "fsl,imx-osc26m"))
 			continue;
 
-		if (!of_property_read_u32(np, "clock-frequency", &fref)) {
-			of_node_put(np);
+		if (!of_property_read_u32(osc_np, "clock-frequency", &fref)) {
+			of_node_put(osc_np);
 			break;
 		}
 	}
 
-	_mx31_clocks_init(fref);
+	ccm = of_iomap(np, 0);
+	if (!ccm)
+		panic("%s: failed to map registers\n", __func__);
 
-	return 0;
+	_mx31_clocks_init(ccm, fref);
+
+	clk_data.clks = clk;
+	clk_data.clk_num = ARRAY_SIZE(clk);
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 }
+
+CLK_OF_DECLARE(imx31_ccm, "fsl,imx31-ccm", mx31_clocks_init_dt);
-- 
2.8.1

^ permalink raw reply related

* [RFC PATCH 0/2] watchdog: imx2+: fix hangup during watchdog initialization
From: Vladimir Zapolskiy @ 2016-09-26  0:39 UTC (permalink / raw)
  To: linux-arm-kernel

It was discovered that the current version of imx2+ watchdog driver
does not work properly on i.MX31 boards and due to the same root
cause the kernel should not work on i.MX21 and i.MX27 powered boards,
if watchdog device driver is probed.

The root cause is that the watchdog controller on all modern SoCs from
i.MX series has two more registers in comparison to the watchdog on
i.MX21, i.MX27 and i.MX31, thus write access to the non-existing
register by the driver probe function causes a core hang.

The commit which caused a regression is 5fe65ce7ccbb ("watchdog:
imx2_wdt: Disable power down counter on boot"), for me it's hard
to say if this change is utterly needed or not (e.g. if it is done
in a bootloader), but it seems to be valid for modern i.MX SoCs.

The solution introduces a runtime difference between i.MX21 and i.MX25
compatible watchdog controllers, also this is reflected in the
updated DTS files.

The change is an RFC, because
* it is based on v4.8-rc1 and it will be rebased after getting review
  comments,
* the selection of compatible watchdog controllers is partially done
  on RMs, but I don't have access to all of them,
* the change fixes legacy boards (prior to DT support), but the fix is
  excessive in sense that WMCR register won't be set on them, to cover
  this case I feel reluctant to add a new header with platform data
  structure declaration for legacy boards, but comments are welcome
* I don't know if a complete list of watchdog compatibles (which is
  expected to be growing with time) is wanted in the driver code,
* if the DT change from the series is added, then only a check for
  fsl,imx25-wdt compatible migth be good enough, but this will
  nullify "disable power down counter" change for boards with
  not updated DTBs.
* note that ls1021a.dtsi does not have its named watchdog compatible,
  I know that Rob insists on adding SoC-named compatibles for
  controllers, this may help in situations like this one, other
  opinions or discussion or fix for LS1021A is welcome.

Vladimir Zapolskiy (2):
  watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
  ARM: i.MX: dts: add fsl,imx25-wdt compatible to all relevant watchdog nodes

 arch/arm/boot/dts/imx35.dtsi   |  3 ++-
 arch/arm/boot/dts/imx50.dtsi   |  3 ++-
 arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
 arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx6sx.dtsi  |  9 +++++---
 arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx7s.dtsi   | 12 +++++++----
 arch/arm/boot/dts/ls1021a.dtsi |  2 +-
 arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
 drivers/watchdog/imx2_wdt.c    | 47 ++++++++++++++++++++++++++++++++++++++++--
 12 files changed, 86 insertions(+), 23 deletions(-)

-- 
2.8.1

^ permalink raw reply

* [RFC PATCH 1/2] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Vladimir Zapolskiy @ 2016-09-26  0:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474850361-20884-1-git-send-email-vz@mleia.com>

Power down counter enable/disable bit switch is located in WMCR
register, but watchdog controllers found on legacy i.MX21, i.MX27 and
i.MX31 SoCs don't have this register. As a result of writing data to
the non-existing register on driver probe the SoC hangs up, to fix the
problem add more OF compatible strings and on this basis get
information about availability of the WMCR register.

Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on boot")
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/watchdog/imx2_wdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 62f346b..b6763e0 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -29,6 +29,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/watchdog.h>
@@ -57,6 +58,10 @@
 
 #define WDOG_SEC_TO_COUNT(s)	((s * 2 - 1) << 8)
 
+struct imx2_wdt_data {
+	bool has_pdc;
+};
+
 struct imx2_wdt_device {
 	struct clk *clk;
 	struct regmap *regmap;
@@ -64,6 +69,8 @@ struct imx2_wdt_device {
 	bool ext_reset;
 };
 
+static const struct of_device_id imx2_wdt_dt_ids[];
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
@@ -200,10 +207,13 @@ static const struct regmap_config imx2_wdt_regmap_config = {
 
 static int __init imx2_wdt_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
+	const struct imx2_wdt_data *data;
 	struct imx2_wdt_device *wdev;
 	struct watchdog_device *wdog;
 	struct resource *res;
 	void __iomem *base;
+	bool has_pdc;
 	int ret;
 	u32 val;
 
@@ -261,12 +271,24 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 		set_bit(WDOG_HW_RUNNING, &wdog->status);
 	}
 
+	if (pdev->dev.of_node) {
+		of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
+		if (!of_id)
+			return -ENODEV;
+
+		data = of_id->data;
+		has_pdc = data->has_pdc;
+	} else {
+		has_pdc = false;
+	}
+
 	/*
 	 * Disable the watchdog power down counter at boot. Otherwise the power
 	 * down counter will pull down the #WDOG interrupt line for one clock
 	 * cycle.
 	 */
-	regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
+	if (has_pdc)
+		regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
 
 	ret = watchdog_register_device(wdog);
 	if (ret) {
@@ -363,8 +385,29 @@ static int imx2_wdt_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
 			 imx2_wdt_resume);
 
+static const struct imx2_wdt_data imx21_wdt_data = {
+	.has_pdc = false,
+};
+
+static const struct imx2_wdt_data imx25_wdt_data = {
+	.has_pdc = true,
+};
+
 static const struct of_device_id imx2_wdt_dt_ids[] = {
-	{ .compatible = "fsl,imx21-wdt", },
+	{ .compatible = "fsl,imx21-wdt",  .data = &imx21_wdt_data },
+	{ .compatible = "fsl,imx25-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx27-wdt",  .data = &imx21_wdt_data },
+	{ .compatible = "fsl,imx31-wdt",  .data = &imx21_wdt_data },
+	{ .compatible = "fsl,imx35-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx50-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx51-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx53-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6q-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6sl-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6sx-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6ul-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx7d-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,vf610-wdt",  .data = &imx25_wdt_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
-- 
2.8.1

^ permalink raw reply related

* [RFC PATCH 2/2] ARM: i.MX: dts: add fsl, imx25-wdt compatible to all relevant watchdog nodes
From: Vladimir Zapolskiy @ 2016-09-26  0:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474850361-20884-1-git-send-email-vz@mleia.com>

Watchdog device controller found on all modern SoCs from i.MX series
and firstly introduced in i.MX25 is not one in one compatible with the
watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
controlles don't have WICR (and pretimeout notification support) and
WMCR registers. To get benefit from the more advanced watchdog device
and to avoid operations over non-existing registers on legacy SoCs add
fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
watchdog controllers.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 arch/arm/boot/dts/imx35.dtsi   |  3 ++-
 arch/arm/boot/dts/imx50.dtsi   |  3 ++-
 arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
 arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx6sx.dtsi  |  9 ++++++---
 arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx7s.dtsi   | 12 ++++++++----
 arch/arm/boot/dts/ls1021a.dtsi |  2 +-
 arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
 11 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 490b7b4..8fd4482 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -284,7 +284,8 @@
 			};
 
 			wdog: wdog at 53fdc000 {
-				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx35-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53fdc000 0x4000>;
 				clocks = <&clks 74>;
 				clock-names = "";
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index e245713..5ba6d5a 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -260,7 +260,8 @@
 			};
 
 			wdog1: wdog at 53f98000 {
-				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx50-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index f46fe9b..d91f713 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -345,14 +345,16 @@
 			};
 
 			wdog1: wdog at 73f98000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x73f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 73f9c000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x73f9c000 0x4000>;
 				interrupts = <59>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index cd17037..c9edac2 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -390,14 +390,16 @@
 			};
 
 			wdog1: wdog at 53f98000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 53f9c000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f9c000 0x4000>;
 				interrupts = <59>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b620ac8..d73edd7 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -593,14 +593,16 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 5425150..b8c71bd 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -430,14 +430,16 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 2863c52..2753c71 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -515,14 +515,16 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
@@ -1178,7 +1180,8 @@
                         };
 
 			wdog3: wdog at 02288000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x02288000 0x4000>;
 				interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 33b95d7..fb0373f 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -483,14 +483,16 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_WDOG1>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_WDOG2>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 1e90bdb..e7c047e 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -394,14 +394,16 @@
 			};
 
 			wdog1: wdog at 30280000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x30280000 0x10000>;
 				interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG1_ROOT_CLK>;
 			};
 
 			wdog2: wdog at 30290000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x30290000 0x10000>;
 				interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG2_ROOT_CLK>;
@@ -409,7 +411,8 @@
 			};
 
 			wdog3: wdog at 302a0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x302a0000 0x10000>;
 				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG3_ROOT_CLK>;
@@ -417,7 +420,8 @@
 			};
 
 			wdog4: wdog at 302b0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x302b0000 0x10000>;
 				interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG4_ROOT_CLK>;
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 368e219..7e36fd8 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -441,7 +441,7 @@
 		};
 
 		wdog0: watchdog at 2ad0000 {
-			compatible = "fsl,imx21-wdt";
+			compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
 			reg = <0x0 0x2ad0000 0x0 0x10000>;
 			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&platform_clk 1>;
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 2c13ec6..35f32ed 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -320,7 +320,8 @@
 			};
 
 			wdoga5: wdog at 4003e000 {
-				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,vf610-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_WDT>;
-- 
2.8.1

^ permalink raw reply related

* [PATCH 0/4] arm/versatile: no-MMU support
From: Greg Ungerer @ 2016-09-26  0:52 UTC (permalink / raw)
  To: linux-arm-kernel

The following patches support configuring and building the versatile
machine with a no-MMU kernel.

There is only a minor few changes required. It was previously possible
in older kernels to build for versatile with CONFIG_MMU disabled, but
the change to devicetree lost that capability. These changes make it
possible again.

One patch is a fix for address translation (broken on older kernels too),
two are build problems when CONFIG_MMU is disabled, and the last is the
actuall configuration changes needed.

The motivation for this is that the versatile machine is well supported
in qemu. And this provides an excellent platform for development and
testing no-MMU support on ARM in general. With these patches applied
it is possible to build and run a kernel with MMU disabled on qemu.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/Kconfig                       |   10 ++++++++++
 arch/arm/Kconfig.debug                 |    3 ++-
 arch/arm/include/asm/mach/map.h        |    1 +
 arch/arm/mach-versatile/Kconfig        |    3 ++-
 arch/arm/mach-versatile/Makefile.boot  |    3 +++
 arch/arm/mach-versatile/versatile_dt.c |    4 ++++
 6 files changed, 22 insertions(+), 2 deletions(-)

^ permalink raw reply

* [PATCH 1/4] arm/versatile: support no-MMU mode addressing
From: Greg Ungerer @ 2016-09-26  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474851150-12532-1-git-send-email-gerg@uclinux.org>

Currently for the versatile boards the IO_ADDRESS() macro applies static
virtual address mapping for built-in IO devices. When operating without
the MMU enabled IO devices are accessed at their physical address, no
address translation is required.

For the !CONFIG_MMU case then define the IO_ADDRESS() macro to return the
physical address.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/mach-versatile/versatile_dt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-versatile/versatile_dt.c b/arch/arm/mach-versatile/versatile_dt.c
index 3c8d39c..8cfa05a 100644
--- a/arch/arm/mach-versatile/versatile_dt.c
+++ b/arch/arm/mach-versatile/versatile_dt.c
@@ -37,7 +37,11 @@
 #include <asm/mach/map.h>
 
 /* macro to get at MMIO space when running virtually */
+#ifdef CONFIG_MMU
 #define IO_ADDRESS(x)		(((x) & 0x0fffffff) + (((x) >> 4) & 0x0f000000) + 0xf0000000)
+#else
+#define IO_ADDRESS(x)		(x)
+#endif
 #define __io_address(n)		((void __iomem __force *)IO_ADDRESS(n))
 
 /*
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/4] arm/versatile: define empty debug_ll_io_init() for no-MMU
From: Greg Ungerer @ 2016-09-26  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474851150-12532-1-git-send-email-gerg@uclinux.org>

No-MMU configured targets have no definition for debug_ll_io_init().
Not all machines use this and it will only be required if CONFIG_DEBUG_LL
is enabled.

But when compiling for a target that uses it and it is configured for
no-MMU (!CONFIG_MMU), for example the versatile machine, you will get:

  CC      arch/arm/mach-versatile/versatile_dt.o
arch/arm/mach-versatile/versatile_dt.c: In function ?versatile_map_io?:
arch/arm/mach-versatile/versatile_dt.c:283:2: error: implicit declaration of function ?debug_ll_io_init? [-Werror=implicit-function-declaration]
  debug_ll_io_init();
  ^

Fix by adding a macro for it to the !CONFIG_MMU path in map.h.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/include/asm/mach/map.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
index 9b7c328..b1fe9c8 100644
--- a/arch/arm/include/asm/mach/map.h
+++ b/arch/arm/include/asm/mach/map.h
@@ -62,6 +62,7 @@ extern int ioremap_page(unsigned long virt, unsigned long phys,
 #else
 #define iotable_init(map,num)	do { } while (0)
 #define vm_reserve_area_early(a,s,c)	do { } while (0)
+#define debug_ll_io_init()	do { } while (0)
 #endif
 
 #endif
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/4] arm/versatile: empty Makefile.boot needed for no-MMU compile
From: Greg Ungerer @ 2016-09-26  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474851150-12532-1-git-send-email-gerg@uclinux.org>

To compile the arm versatile board with the MMU disabled (!CONFIG_MMU)
a Makefile.boot is required. Without it you get:

  SYSMAP  System.map
arch/arm/boot/Makefile:15: arch/arm/mach-versatile//Makefile.boot: No such file or directory
make[2]: *** No rule to make target `arch/arm/mach-versatile//Makefile.boot'.  Stop.

Create an empty Makefile.boot for the versatile machine. This is a
copy of the other empty machine Makefile.boot files. (A few have this
same commented empty file: stm32, ep93xx, lpc18xx, efm32 and vexpress).

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/mach-versatile/Makefile.boot | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 arch/arm/mach-versatile/Makefile.boot

diff --git a/arch/arm/mach-versatile/Makefile.boot b/arch/arm/mach-versatile/Makefile.boot
new file mode 100644
index 0000000..eacfc3f
--- /dev/null
+++ b/arch/arm/mach-versatile/Makefile.boot
@@ -0,0 +1,3 @@
+# Empty file waiting for deletion once Makefile.boot isn't needed any more.
+# Patch waits for application at
+# http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7889/1 .
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/4] arm/versatile: support configuring versatile machine for no-MMU
From: Greg Ungerer @ 2016-09-26  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474851150-12532-1-git-send-email-gerg@uclinux.org>

Allow the arm versatile machine to be configured for no-MMU operation.
The approach taken is similar to the support for no-MMU arm v7 machines.

A new define, CONFIG_ARM_SINGLE_ARMV5, is used to enable a class of v5
core based machines that are supported for building with !CONFIG_MMU.
Currently only the versatile machine is configured to support this.

Older kernels had the ability to build the versatile machine with the MMU
disabled (!CONFIG_MMU). Recent changes to convert the versatile machine
to device tree lost this ability. (Although older kernels could be built
they did not run due to a bug in the IO_ADDRESS() mapping on this machine).

The motivation for this is that the versatile machine is well supported
in qemu. And this provides an excellent platform for development and
testing no-MMU support on ARM in general.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/Kconfig                | 10 ++++++++++
 arch/arm/Kconfig.debug          |  3 ++-
 arch/arm/mach-versatile/Kconfig |  3 ++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a9c4e48..f44fe7b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -353,6 +353,16 @@ config ARM_SINGLE_ARMV7M
 	select SPARSE_IRQ
 	select USE_OF
 
+config ARM_SINGLE_ARMV5
+	bool "ARMv5 based platforms (ARM926T)"
+	depends on !MMU
+	select AUTO_ZRELADDR
+	select CLKSRC_OF
+	select COMMON_CLK
+	select GENERIC_CLOCKEVENTS
+	select SPARSE_IRQ
+	select USE_OF
+
 config ARCH_GEMINI
 	bool "Cortina Systems Gemini"
 	select CLKSRC_MMIO
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index a9693b6..9897348 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1712,7 +1712,8 @@ config DEBUG_UNCOMPRESS
 config UNCOMPRESS_INCLUDE
 	string
 	default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \
-					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M
+					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
+					ARM_SINGLE_ARMV5
 	default "mach/uncompress.h"
 
 config EARLY_PRINTK
diff --git a/arch/arm/mach-versatile/Kconfig b/arch/arm/mach-versatile/Kconfig
index b0cc262..904a012 100644
--- a/arch/arm/mach-versatile/Kconfig
+++ b/arch/arm/mach-versatile/Kconfig
@@ -1,6 +1,6 @@
 config ARCH_VERSATILE
 	bool "ARM Ltd. Versatile family"
-	depends on ARCH_MULTI_V5
+	depends on ARCH_MULTI_V5 || ARM_SINGLE_ARMV5
 	select ARM_AMBA
 	select ARM_TIMER_SP804
 	select ARM_VIC
@@ -14,6 +14,7 @@ config ARCH_VERSATILE
 	select POWER_RESET
 	select POWER_RESET_VERSATILE
 	select VERSATILE_FPGA_IRQ
+	select GPIOLIB
 	help
 	  This enables support for ARM Ltd Versatile board.
 
-- 
1.9.1

^ permalink raw reply related

* [v12, 0/8] Fix eSDHC host version register bug
From: Y.B. Lu @ 2016-09-26  3:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474441040-11946-1-git-send-email-yangbo.lu@nxp.com>

Any comments about this version patchset ?

:)


> -----Original Message-----
> From: Yangbo Lu [mailto:yangbo.lu at nxp.com]
> Sent: Wednesday, September 21, 2016 2:57 PM
> To: linux-mmc at vger.kernel.org; ulf.hansson at linaro.org; Scott Wood; Arnd
> Bergmann
> Cc: linuxppc-dev at lists.ozlabs.org; devicetree at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; linux-kernel at vger.kernel.org; linux-
> clk at vger.kernel.org; linux-i2c at vger.kernel.org; iommu at lists.linux-
> foundation.org; netdev at vger.kernel.org; Mark Rutland; Rob Herring;
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie; M.H.
> Lian; Y.B. Lu
> Subject: [v12, 0/8] Fix eSDHC host version register bug
> 
> This patchset is used to fix a host version register bug in the T4240-
> R1.0-R2.0 eSDHC controller. To match the SoC version and revision, 10
> previous version patchsets had tried many methods but all of them were
> rejected by reviewers.
> Such as
> 	- dts compatible method
> 	- syscon method
> 	- ifdef PPC method
> 	- GUTS driver getting SVR method
> Anrd suggested a soc_device_match method in v10, and this is the only
> available method left now. This v11 patchset introduces the
> soc_device_match interface in soc driver.
> 
> The first six patches of Yangbo are to add the GUTS driver. This is used
> to register a soc device which contain soc version and revision
> information.
> The other two patches introduce the soc_device_match method in soc driver
> and apply it on esdhc driver to fix this bug.
> 
> Arnd Bergmann (1):
>   base: soc: introduce soc_device_match() interface
> 
> Yangbo Lu (7):
>   dt: bindings: update Freescale DCFG compatible
>   ARM64: dts: ls2080a: add device configuration node
>   dt: bindings: move guts devicetree doc out of powerpc directory
>   powerpc/fsl: move mpc85xx.h to include/linux/fsl
>   soc: fsl: add GUTS driver for QorIQ platforms
>   MAINTAINERS: add entry for Freescale SoC drivers
>   mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
> 
>  Documentation/devicetree/bindings/arm/fsl.txt      |   6 +-
>  .../bindings/{powerpc => soc}/fsl/guts.txt         |   3 +
>  MAINTAINERS                                        |  11 +-
>  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi     |   6 +
>  arch/powerpc/kernel/cpu_setup_fsl_booke.S          |   2 +-
>  arch/powerpc/sysdev/fsl_pci.c                      |   2 +-
>  drivers/base/Kconfig                               |   1 +
>  drivers/base/soc.c                                 |  66 ++++++
>  drivers/clk/clk-qoriq.c                            |   3 +-
>  drivers/i2c/busses/i2c-mpc.c                       |   2 +-
>  drivers/iommu/fsl_pamu.c                           |   3 +-
>  drivers/mmc/host/Kconfig                           |   1 +
>  drivers/mmc/host/sdhci-of-esdhc.c                  |  20 ++
>  drivers/net/ethernet/freescale/gianfar.c           |   2 +-
>  drivers/soc/Kconfig                                |   2 +-
>  drivers/soc/fsl/Kconfig                            |  19 ++
>  drivers/soc/fsl/Makefile                           |   1 +
>  drivers/soc/fsl/guts.c                             | 257
> +++++++++++++++++++++
>  include/linux/fsl/guts.h                           | 125 ++++++----
>  .../asm/mpc85xx.h => include/linux/fsl/svr.h       |   4 +-
>  include/linux/sys_soc.h                            |   3 +
>  21 files changed, 478 insertions(+), 61 deletions(-)  rename
> Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
> create mode 100644 drivers/soc/fsl/Kconfig  create mode 100644
> drivers/soc/fsl/guts.c  rename arch/powerpc/include/asm/mpc85xx.h =>
> include/linux/fsl/svr.h (97%)
> 
> --
> 2.1.0.27.g96db324

^ permalink raw reply

* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
From: Peter Chen @ 2016-09-26  3:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <147457026249.9057.8712383708019243942@sboyd-linaro>

On Thu, Sep 22, 2016 at 11:51:02AM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-16 18:16:05)
> > On Wed, Sep 14, 2016 at 01:55:02AM -0700, Stephen Boyd wrote:
> > > Quoting Stephen Boyd (2016-09-13 18:42:46)
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > > > 
> > > > [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
> > > > 
> > > > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > > > Cc: <devicetree@vger.kernel.org>
> > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > ---
> > > > 
> > > > Should I make the extcon part optional? I could see a case where there are two
> > > > "OTG" ports connected to the mux (or two hubs), and for some reason the
> > > > software may want to mux between them at runtime. If we mandate an extcon,
> > > > that won't be possible to support. Perhaps it would be better to have
> > > > the node, but connect it to the usb controller with a phandle (maybe of_graph
> > > > endpoints would be useful too) so that when the controller wants to mux over
> > > > a port it can do so.
> > > 
> > > Here's some dts mock-up on top of the db410c for the of_graph stuff. I
> > > haven't written any code around it, but the idea is to allow the binding
> > > to specify how the mux is connected to upstream and downstream D+/D-
> > > lines. This way, we can do some dt parsing of the endpoints and their
> > > parent nodes to figure out if the mux needs to be set high or low to use
> > > a device connector or a usb hub based on if the id cable is present.
> > > Maybe I'm over thinking things though and we could just have a DT
> > > property for that.
> > > 
> > >       soc {
> > >               usb at 78d9000 {
> > >                       extcon = <&usb_id>, <&usb_id>;
> > 
> > Why you have two same extcon phandler? From my mind, one should id,
> > another should is vbus. Besides, I find extcon-usb-gpio.c is lack of
> > vbus support, how you support vbus detection for
> > connection/disconnection with PC for your chipidea msm patch set?
> 
> This was already in the dts files for db410c. In the chipidea binding
> one is for EXTCON_USB (vbus) and one is for EXTCON_USB_HOST (id). My
> understanding is that extcon-usb-gpio.c sends events for both EXTCON_USB
> and EXTCON_USB_HOST when the gpio changes state. vbus detection is not
> that great on this board because we only have on gpio for this.

I think extcon-usb-gpio.c needs to extend for supporting vbus event,
otherwise, the micro-b cable's connect/disconnect will introduce
EXTCON_USB_HOST event, if you use two <&usb_idx> for both id and
vbus event.

-- 

Best Regards,
Peter Chen

^ 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