Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] sound: soc: codecs: Add es8328 codec
From: Mark Brown @ 2014-02-10 14:23 UTC (permalink / raw)
  To: Sean Cross
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Shawn Guo, Sascha Hauer,
	Liam Girdwood
In-Reply-To: <52F5E661.5010807-nXMMniAx+RbQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3534 bytes --]

On Sat, Feb 08, 2014 at 04:10:09PM +0800, Sean Cross wrote:
> On 8/2/14 2:12 AM, Mark Brown wrote:

> > In general this looks like it should be making much more use of the
> > framework rather than open coding, it looks like it's very much hard
> > coded for one use cae.

> Alright, I'll try to use more of the framework provided.  I tried to use
> one of the Wolfson codecs as an example.  Is there a "canonical" driver
> that's up-to-date and uses an appreciable amount of framework?

Try wm8731 I guess, something similar to the device, there's a lot of
variation in device complexity and several generations of hardware?  I'm
not sure what driver you were looking at here.

> >> +static const struct snd_soc_dapm_widget es8328_dapm_widgets[] = {
> >> +    SND_SOC_DAPM_DAC("Speaker Volume", "HiFi Playback",
> SND_SOC_NOPM, 0, 0),

> > Don't declare a stream by name, use DAPM routes to connect the stream to
> > the widget.

> Where can I read up on DAPM routes?  I don't see the word "route"
> mentioned in the dapm.txt SoC documentation.

Paths.

> >> +static int es8328_adc_enable(struct snd_soc_codec *codec)
> >> +{
> >> +    u16 reg = snd_soc_read(codec, ES8328_CHIPPOWER);
> >> +    reg &= ~(ES8328_CHIPPOWER_ADCVREF_OFF |
> >> +         ES8328_CHIPPOWER_ADCPLL_OFF |
> >> +         ES8328_CHIPPOWER_ADCSTM_RESET |
> >> +         ES8328_CHIPPOWER_ADCDIG_OFF);
> >> +    snd_soc_write(codec, ES8328_CHIPPOWER, reg);

> > This looks like it should be done in DAPM.

> The codec has very strict power sequencing.  If I'm understanding the
> document correctly, I'd add a DAPM widget for the DAC, and then register
> a widget event for when the DAC gets opened, and do power sequencing
> there?  Or is there a way to have multiple register writes defined as a
> single DAPM widget?

An event callback is the way to go.  However please check what the
datasheet is actually trying to say - I suspect you will find it's
giving you a procedure that's got common elements for core power up and
then is saying something like "keep the outputs muted while you power
them up" but is doing it with lots of examples.

> >> +    /* Frequency clock of 272 */
> >> +    snd_soc_write(codec, ES8328_ADCCONTROL5, 0x02);

> > What is the frequency clock in this context?

> The frequency is a specific divider defined in the datasheet.  There is
> a table of frequencies and dividers for various frequencies.  One option
> is to code this table into the driver and pick the frequency clocks out
> of the table.

That seems much better.

> I decided to assume the codec has a fixed input frequency of 22.5792
> MHz.  This means the codec can support 11.025, 22.05, and 44.1 kHz, but
> cannot support 48 kHz.  It should be possible to instead feed the codec
> a 24 MHz clock and run it at 48 kHz.  Pulseaudio runs at 44.1 by
> default, so I went with that.

This is what set_sysclk() is for, letting the driver know what the clock
rate is on the current board (if you are willing to go DT only you can
just use the clock API).

> My fault for omitting one.
> Much of the code in adc_enable() and dac_enable() come from the
> reference manual.  They indicate that the codec has very strict ordering
> requirements on registers that must be set during playback and
> recording.  Again, is there any way to ensure that hw_params(),
> set_dai_fmt(), and the DAPM widgets are called in a specific order?

No, there's no way to do that.  Like I say I am somewhat mistrustful of
the clarity/accuracy of the hardware limitations being described here.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices
From: Mark Brown @ 2014-02-10 14:14 UTC (permalink / raw)
  To: Lee Jones
  Cc: Adam Thomson, alsa-devel, linux-kernel, devicetree, Rob Herring,
	Linus Walleij, Dmitry Torokhov, Alessandro Zummo, Guenter Roeck
In-Reply-To: <20140210135939.GA23141@lee--X1>

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

On Mon, Feb 10, 2014 at 01:59:39PM +0000, Lee Jones wrote:

> > Applied, thanks.

> Actually, do you mind if I take this and apply it to an immutable?

> I'd rather not be on the wrong end of some wrath, due to not playing
> the game properly.

OK, but it does need to go in as a fix targetted for stable.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFC 1/6] mailbox: add core framework
From: Arnd Bergmann @ 2014-02-10 14:11 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: s-anna, rob.herring, rafael.j.wysocki, mark.langsdorf, tony,
	omar.ramirez, gregkh, pawel.moll, mark.rutland, ijc+devicetree,
	galak, rob, linux-doc, devicetree, linux-kernel
In-Reply-To: <1391820619-25487-2-git-send-email-courtney.cavin@sonymobile.com>

On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> The mailbox drivers are fragmented, and some implement their own core.
> Unify the drivers and implement common functionality in a framework.
> 
> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>

This seems pretty cool overall, great to see someone getting at it@

> +static void of_mbox_adapter_add(struct mbox_adapter *adap)
> +{
> +	if (!adap->dev)
> +		return;
> +
> +	if (!adap->of_xlate) {
> +		adap->of_xlate = of_mbox_simple_xlate;
> +		adap->of_n_cells = 1;
> +	}
> +
> +	of_node_get(adap->dev->of_node);
> +}

You should probably check if of_n_cells matches the device node
#mbox-cells value, otherwise the xlate function will get confused.

> +
> +	mutex_lock(&mbox_lock);
> +	list_add(&adap->list, &mbox_adapters);
> +
> +	of_mbox_adapter_add(adap);
> +	mutex_unlock(&mbox_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mbox_adapter_add);

Please use EXPORT_SYMBOL_GPL here and elsewhere.

> +/**
> + * mbox_channel_notify() - notify the core that a channel has a message
> + * @chan: the channel which has data
> + * @data: the location of said data
> + * @len: the length of specified data
> + *
> + * This function may be called from interrupt/no-sleep context.
> + */
> +int mbox_channel_notify(struct mbox_channel *chan,
> +		const void *data, unsigned int len)
> +{
> +	return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> +}
> +EXPORT_SYMBOL(mbox_channel_notify);

What is the reason to use a notifier chain here? Isn't a simple
callback function pointer enough? I would expect that each mailbox
can have exactly one consumer, not multiple ones.

> +/**
> + * mbox_add_table() - add a lookup table for adapter consumers
> + * @table: array of consumers to register
> + * @num: number of consumers in array
> + */
> +void __init mbox_add_table(struct mbox_lookup *table, unsigned int num)
> +{
> +	mutex_lock(&mbox_lookup_lock);
> +	while (num--) {
> +		if (table->provider && (table->dev_id || table->con_id))
> +			list_add_tail(&table->list, &mbox_lookup_list);
> +		table++;
> +	}
> +	mutex_unlock(&mbox_lookup_lock);
> +}
> +EXPORT_SYMBOL(mbox_add_table);

I don't understand this part of the API. Why do you need a separate
lookup table here? Isn't that what the DT lookup does already?

> +/**
> + * mbox_request() - lookup and request a MBOX channel
> + * @dev: device for channel consumer
> + * @con_id: consumer name
> + * @nb: notifier block used for receiving messages
> + *
> + * The notifier is called as atomic on new messages, so you may not sleep
> + * in the notifier callback function.
> + */
> +struct mbox *mbox_request(struct device *dev, const char *con_id,
> +		struct notifier_block *nb)
> +{
> +	struct mbox_adapter *adap;
> +	struct mbox_channel *chan;
> +	struct mbox *mbox;
> +	int index = 0;
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> +		return of_mbox_request(dev->of_node, con_id, nb);

What use case do you have in mind for !CONFIG_OF?

> +/**
> + * struct mbox_adapter_ops - MBOX adapter operations
> + * @put_message: hook for putting messages in the channels MBOX
> + * @request: optional hook for requesting an MBOX channel
> + * @release: optional hook for releasing an MBOX channel
> + * @owner: helps prevent removal of modules exporting active MBOX channels
> + */
> +struct mbox_adapter_ops {
> +	int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
> +				const void *, unsigned int);
> +	int (*request)(struct mbox_adapter *, struct mbox_channel *);
> +	int (*release)(struct mbox_adapter *, struct mbox_channel *);
> +	struct module *owner;
> +};

I think we will need a peek_message() callback for the upcoming
QMTM driver, to allow client drivers to get a message out before
the mailbox driver gets an IRQ. This will be used for IRQ mitigation
in the network driver.

	Arnd

^ permalink raw reply

* Re: [PATCH v2 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices
From: Lee Jones @ 2014-02-10 13:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: Adam Thomson, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Linus Walleij,
	Dmitry Torokhov, Alessandro Zummo, Guenter Roeck
In-Reply-To: <20140210134310.GH1757-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

> > Currently the I2C device Ids conflict for the MFD and CODEC so
> > cannot be both instantiated on one platform. This patch updates
> > the Ids and names to make them unique from each other.
> 
> Applied, thanks.

Actually, do you mind if I take this and apply it to an immutable?

I'd rather not be on the wrong end of some wrath, due to not playing
the game properly.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices
From: Mark Brown @ 2014-02-10 13:43 UTC (permalink / raw)
  To: Adam Thomson
  Cc: devicetree, alsa-devel, Alessandro Zummo, Linus Walleij,
	Dmitry Torokhov, linux-kernel, Rob Herring, Lee Jones,
	Guenter Roeck
In-Reply-To: <6a398d176b0b2bf8792d27bd5e2995fd96afb32d.1391705989.git.Adam.Thomson.Opensource@diasemi.com>


[-- Attachment #1.1: Type: text/plain, Size: 268 bytes --]

On Thu, Feb 06, 2014 at 06:03:07PM +0000, Adam Thomson wrote:
> Currently the I2C device Ids conflict for the MFD and CODEC so
> cannot be both instantiated on one platform. This patch updates
> the Ids and names to make them unique from each other.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [PATCH] dp83640: Get gpio and master/slave configuration from DT
From: Mark Rutland @ 2014-02-10 13:42 UTC (permalink / raw)
  To: Stefan Sørensen
  Cc: richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1392037240-30913-1-git-send-email-stefan.sorensen-usnHOLptxrsHrNJx0XZkJA@public.gmane.org>

On Mon, Feb 10, 2014 at 01:00:40PM +0000, Stefan Sørensen wrote:
> This patch removes the module parameters gpio_tab and chosen_phy in favour of
> retrieving the configuration from DT through the properties
> 	dp83640,slave
> 	dp83640,calibrate-gpio
> 	dp83640,perout-gpios
> 	dp83640,extts-gpios
> The configuration is now stored for each master clock device, allowing different 
> gpio setups for each master.
> 
> Furthermore the code is enhanced to handle more than one periodic output.

Binding document please.

I have some basic comments below, but without a description of what the
properties mean, it's difficult to provide any meaningful review.

[...]

> +static int dp83640_probe_dt(struct device_node *node,
> +			    struct dp83640_private *dp83640)
> +{
> +	struct dp83640_clock *clock = dp83640->clock;
> +	struct property *prop;
> +	int err, proplen;
> +
> +	if (!node)
> +		return 0;
> +
> +	if (of_find_property(node, "dp83640,slave", NULL))
> +		dp83640->slave = true;

Use of_property_read_bool.

> +	if (!dp83640->slave && clock->chosen) {
> +		pr_err("dp83640,slave must be set if more than one device on the same bus");
> +		return -EINVAL;
> +	}
> +
> +	prop = of_find_property(node, "dp83640,perout-gpios", &proplen);
> +	if (prop) {
> +		if (dp83640->slave) {
> +			pr_err("dp83640,perout-gpios property can not be set together with dp83640,slave");
> +			return -EINVAL;
> +		}
> +
> +		clock->caps.n_per_out = proplen / sizeof(u32);
> +		if (clock->caps.n_per_out > N_EXT) {
> +			pr_err("dp83640,perout-gpios may not have more than %d entries",
> +			       N_EXT);
> +			return -EINVAL;
> +		}
> +		err = of_property_read_u32_array(node, "dp83640,perout-gpios",
> +						 clock->perout_gpios,
> +						 clock->caps.n_per_out);
> +		if (err < 0)
> +			return err;
> +	}

This looks nothing like the standard gpio bindings. What _exactly_ is
this property describing?

If this is not using the standard gpio bindings then this should be
renamed.

Either way this must be documented.

> +
> +	prop = of_find_property(node, "dp83640,extts-gpios", &proplen);
> +	if (prop) {
> +		if (dp83640->slave) {
> +			pr_err("dp83640,extts-gpios property can not be set together with dp83640,slave");
> +			return -EINVAL;
> +		}
> +
> +		clock->caps.n_ext_ts = proplen / sizeof(u32);
> +		if (clock->caps.n_ext_ts > N_EXT) {
> +			pr_err("dp83640,extts-gpios may not have more than %d entries",
> +			       N_EXT);
> +			return -EINVAL;
> +		}
> +		err = of_property_read_u32_array(node, "dp83640,extts-gpios",
> +						 clock->extts_gpios,
> +						 clock->caps.n_ext_ts);
> +		if (err < 0)
> +			return err;
> +	}

Similarly this does not look right for parsing a standard -gpios property.

> +
> +	prop = of_find_property(node, "dp83640,calibrate-gpio", &proplen);
> +	if (prop) {
> +		if (dp83640->slave) {
> +			pr_err("dp83640,calibrate-gpio property can not be set together with dp83640,slave");
> +			return -EINVAL;
> +		}
> +		clock->calibrate_gpio = -1;
> +		of_property_read_u32(node, "dp83640,calibrate-gpio",
> +				     &clock->calibrate_gpio);
> +	}

And again, this doesn't look right.

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

^ permalink raw reply

* Re: [PATCH 1/2] ARM: shmobile: r8a7791: add Ether DT support
From: Sergei Shtylyov @ 2014-02-10 13:24 UTC (permalink / raw)
  To: Ben Dooks
  Cc: horms, linux-sh, devicetree, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, magnus.damm, linux, linux-arm-kernel
In-Reply-To: <52F8CBFB.5060707@codethink.co.uk>

Hello.

On 10-02-2014 16:54, Ben Dooks wrote:

>> Define the generic R8A7791 part of the Ether device node.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>>   arch/arm/boot/dts/r8a7791.dtsi |   15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)

>> Index: renesas/arch/arm/boot/dts/r8a7791.dtsi
>> ===================================================================
>> --- renesas.orig/arch/arm/boot/dts/r8a7791.dtsi
>> +++ renesas/arch/arm/boot/dts/r8a7791.dtsi
>> @@ -2,7 +2,8 @@
>>    * Device Tree Source for the r8a7791 SoC
>>    *
>>    * Copyright (C) 2013 Renesas Electronics Corporation
>> - * Copyright (C) 2013 Renesas Solutions Corp.
>> + * Copyright (C) 2013-2014 Renesas Solutions Corp.
>> + * Copyright (C) 2014 Cogent Embedded Inc.
>>    *
>>    * This file is licensed under the terms of the GNU General Public License
>>    * version 2.  This program is licensed "as is" without any warranty of any
>> @@ -366,6 +367,18 @@
>>           status = "disabled";
>>       };
>>
>> +    ether: ethernet@ee700000 {
>> +        compatible = "renesas,ether-r8a7791";
>> +        reg = <0 0xee700000 0 0x400>;
>> +        interrupt-parent = <&gic>;
>> +        interrupts = <0 162 IRQ_TYPE_LEVEL_HIGH>;
>> +        clocks = <&mstp8_clks R8A7791_CLK_ETHER>;
>> +        phy-mode = "rmii";
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        status = "disabled";
>> +    };

> Is the device only capable of doing RMII?

    Yes.

WBR, Sergei


^ permalink raw reply

* [PATCH] dp83640: Get gpio and master/slave configuration from DT
From: Stefan Sørensen @ 2014-02-10 13:00 UTC (permalink / raw)
  To: richardcochran, grant.likely, robh+dt, netdev, linux-kernel,
	devicetree
  Cc: Stefan Sørensen

This patch removes the module parameters gpio_tab and chosen_phy in favour of
retrieving the configuration from DT through the properties
	dp83640,slave
	dp83640,calibrate-gpio
	dp83640,perout-gpios
	dp83640,extts-gpios
The configuration is now stored for each master clock device, allowing different 
gpio setups for each master.

Furthermore the code is enhanced to handle more than one periodic output.

Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---

The slave configuration code is totally untested as I only have access to 
boards with a single DP3640. 

 drivers/net/phy/dp83640.c | 195 ++++++++++++++++++++++++++++++----------------
 1 file changed, 128 insertions(+), 67 deletions(-)

diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 547725f..118403d 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -30,6 +30,7 @@
 #include <linux/phy.h>
 #include <linux/ptp_classify.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/of_device.h>
 
 #include "dp83640_reg.h"
 
@@ -38,15 +39,11 @@
 #define LAYER4		0x02
 #define LAYER2		0x01
 #define MAX_RXTS	64
-#define N_EXT_TS	6
+#define N_EXT		8
 #define PSF_PTPVER	2
 #define PSF_EVNT	0x4000
 #define PSF_RX		0x2000
 #define PSF_TX		0x1000
-#define EXT_EVENT	1
-#define CAL_EVENT	7
-#define CAL_TRIGGER	7
-#define PER_TRIGGER	6
 
 #define MII_DP83640_MICR 0x11
 #define MII_DP83640_MISR 0x12
@@ -123,6 +120,8 @@ struct dp83640_private {
 	/* queues of incoming and outgoing packets */
 	struct sk_buff_head rx_queue;
 	struct sk_buff_head tx_queue;
+	/* is this phyter a slave */
+	bool slave;
 };
 
 struct dp83640_clock {
@@ -144,35 +143,9 @@ struct dp83640_clock {
 	struct list_head phylist;
 	/* reference to our PTP hardware clock */
 	struct ptp_clock *ptp_clock;
+	u32 perout_gpios[N_EXT], extts_gpios[N_EXT], calibrate_gpio;
 };
 
-/* globals */
-
-enum {
-	CALIBRATE_GPIO,
-	PEROUT_GPIO,
-	EXTTS0_GPIO,
-	EXTTS1_GPIO,
-	EXTTS2_GPIO,
-	EXTTS3_GPIO,
-	EXTTS4_GPIO,
-	EXTTS5_GPIO,
-	GPIO_TABLE_SIZE
-};
-
-static int chosen_phy = -1;
-static ushort gpio_tab[GPIO_TABLE_SIZE] = {
-	1, 2, 3, 4, 8, 9, 10, 11
-};
-
-module_param(chosen_phy, int, 0444);
-module_param_array(gpio_tab, ushort, NULL, 0444);
-
-MODULE_PARM_DESC(chosen_phy, \
-	"The address of the PHY to use for the ancillary clock features");
-MODULE_PARM_DESC(gpio_tab, \
-	"Which GPIO line to use for which purpose: cal,perout,extts1,...,extts6");
-
 /* a list of clocks and a mutex to protect it */
 static LIST_HEAD(phyter_clocks);
 static DEFINE_MUTEX(phyter_clocks_lock);
@@ -267,15 +240,16 @@ static u64 phy2txts(struct phy_txts *p)
 }
 
 static void periodic_output(struct dp83640_clock *clock,
-			    struct ptp_clock_request *clkreq, bool on)
+			    struct ptp_clock_request *clkreq, int index,
+			    bool on)
 {
 	struct dp83640_private *dp83640 = clock->chosen;
 	struct phy_device *phydev = dp83640->phydev;
 	u32 sec, nsec, period;
 	u16 gpio, ptp_trig, trigger, val;
 
-	gpio = on ? gpio_tab[PEROUT_GPIO] : 0;
-	trigger = PER_TRIGGER;
+	gpio = on ? clock->perout_gpios[index] : 0;
+	trigger = clock->caps.n_ext_ts + index;
 
 	ptp_trig = TRIG_WR |
 		(trigger & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT |
@@ -430,12 +404,12 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		index = rq->extts.index;
-		if (index < 0 || index >= N_EXT_TS)
+		if (index < 0 || index >= clock->caps.n_ext_ts)
 			return -EINVAL;
-		event_num = EXT_EVENT + index;
+		event_num = index;
 		evnt = EVNT_WR | (event_num & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
 		if (on) {
-			gpio_num = gpio_tab[EXTTS0_GPIO + index];
+			gpio_num = clock->extts_gpios[index];
 			evnt |= (gpio_num & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 			evnt |= EVNT_RISE;
 		}
@@ -443,9 +417,10 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
 		return 0;
 
 	case PTP_CLK_REQ_PEROUT:
-		if (rq->perout.index != 0)
+		index = rq->perout.index;
+		if (index < 0 || index >= clock->caps.n_per_out)
 			return -EINVAL;
-		periodic_output(clock, rq, on);
+		periodic_output(clock, rq, index, on);
 		return 0;
 
 	default:
@@ -538,10 +513,9 @@ static void recalibrate(struct dp83640_clock *clock)
 	struct list_head *this;
 	struct dp83640_private *tmp;
 	struct phy_device *master = clock->chosen->phydev;
-	u16 cal_gpio, cfg0, evnt, ptp_trig, trigger, val;
+	u16 cfg0, evnt, ptp_trig, trigger, val;
 
-	trigger = CAL_TRIGGER;
-	cal_gpio = gpio_tab[CALIBRATE_GPIO];
+	trigger = clock->caps.n_ext_ts + clock->caps.n_per_out;
 
 	mutex_lock(&clock->extreg_lock);
 
@@ -564,8 +538,8 @@ static void recalibrate(struct dp83640_clock *clock)
 	 * enable an event timestamp
 	 */
 	evnt = EVNT_WR | EVNT_RISE | EVNT_SINGLE;
-	evnt |= (CAL_EVENT & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
-	evnt |= (cal_gpio & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
+	evnt |= (trigger & EVNT_SEL_MASK) << EVNT_SEL_SHIFT;
+	evnt |= (clock->calibrate_gpio & EVNT_GPIO_MASK) << EVNT_GPIO_SHIFT;
 
 	list_for_each(this, &clock->phylist) {
 		tmp = list_entry(this, struct dp83640_private, list);
@@ -578,7 +552,7 @@ static void recalibrate(struct dp83640_clock *clock)
 	 */
 	ptp_trig = TRIG_WR | TRIG_IF_LATE | TRIG_PULSE;
 	ptp_trig |= (trigger  & TRIG_CSEL_MASK) << TRIG_CSEL_SHIFT;
-	ptp_trig |= (cal_gpio & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
+	ptp_trig |= (clock->calibrate_gpio & TRIG_GPIO_MASK) << TRIG_GPIO_SHIFT;
 	ext_write(0, master, PAGE5, PTP_TRIG, ptp_trig);
 
 	/* load trigger */
@@ -642,7 +616,7 @@ static void recalibrate(struct dp83640_clock *clock)
 
 static inline u16 exts_chan_to_edata(int ch)
 {
-	return 1 << ((ch + EXT_EVENT) * 2);
+	return 1 << ((ch) * 2);
 }
 
 static int decode_evnt(struct dp83640_private *dp83640,
@@ -676,14 +650,14 @@ static int decode_evnt(struct dp83640_private *dp83640,
 		parsed = words + 2;
 	} else {
 		parsed = words + 1;
-		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK) - EXT_EVENT;
+		i = ((ests >> EVNT_NUM_SHIFT) & EVNT_NUM_MASK);
 		ext_status = exts_chan_to_edata(i);
 	}
 
 	event.type = PTP_CLOCK_EXTTS;
 	event.timestamp = phy2txts(&dp83640->edata);
 
-	for (i = 0; i < N_EXT_TS; i++) {
+	for (i = 0; i < dp83640->clock->caps.n_ext_ts; i++) {
 		if (ext_status & exts_chan_to_edata(i)) {
 			event.index = i;
 			ptp_clock_event(dp83640->clock->ptp_clock, &event);
@@ -889,8 +863,6 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	sprintf(clock->caps.name, "dp83640 timer");
 	clock->caps.max_adj	= 1953124;
 	clock->caps.n_alarm	= 0;
-	clock->caps.n_ext_ts	= N_EXT_TS;
-	clock->caps.n_per_out	= 1;
 	clock->caps.pps		= 0;
 	clock->caps.adjfreq	= ptp_dp83640_adjfreq;
 	clock->caps.adjtime	= ptp_dp83640_adjtime;
@@ -903,18 +875,6 @@ static void dp83640_clock_init(struct dp83640_clock *clock, struct mii_bus *bus)
 	get_device(&bus->dev);
 }
 
-static int choose_this_phy(struct dp83640_clock *clock,
-			   struct phy_device *phydev)
-{
-	if (chosen_phy == -1 && !clock->chosen)
-		return 1;
-
-	if (chosen_phy == phydev->addr)
-		return 1;
-
-	return 0;
-}
-
 static struct dp83640_clock *dp83640_clock_get(struct dp83640_clock *clock)
 {
 	if (clock)
@@ -960,6 +920,100 @@ static void dp83640_clock_put(struct dp83640_clock *clock)
 	mutex_unlock(&clock->clock_lock);
 }
 
+#ifdef CONFIG_OF
+static int dp83640_probe_dt(struct device_node *node,
+			    struct dp83640_private *dp83640)
+{
+	struct dp83640_clock *clock = dp83640->clock;
+	struct property *prop;
+	int err, proplen;
+
+	if (!node)
+		return 0;
+
+	if (of_find_property(node, "dp83640,slave", NULL))
+		dp83640->slave = true;
+	if (!dp83640->slave && clock->chosen) {
+		pr_err("dp83640,slave must be set if more than one device on the same bus");
+		return -EINVAL;
+	}
+
+	prop = of_find_property(node, "dp83640,perout-gpios", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640,perout-gpios property can not be set together with dp83640,slave");
+			return -EINVAL;
+		}
+
+		clock->caps.n_per_out = proplen / sizeof(u32);
+		if (clock->caps.n_per_out > N_EXT) {
+			pr_err("dp83640,perout-gpios may not have more than %d entries",
+			       N_EXT);
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,perout-gpios",
+						 clock->perout_gpios,
+						 clock->caps.n_per_out);
+		if (err < 0)
+			return err;
+	}
+
+	prop = of_find_property(node, "dp83640,extts-gpios", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640,extts-gpios property can not be set together with dp83640,slave");
+			return -EINVAL;
+		}
+
+		clock->caps.n_ext_ts = proplen / sizeof(u32);
+		if (clock->caps.n_ext_ts > N_EXT) {
+			pr_err("dp83640,extts-gpios may not have more than %d entries",
+			       N_EXT);
+			return -EINVAL;
+		}
+		err = of_property_read_u32_array(node, "dp83640,extts-gpios",
+						 clock->extts_gpios,
+						 clock->caps.n_ext_ts);
+		if (err < 0)
+			return err;
+	}
+
+	prop = of_find_property(node, "dp83640,calibrate-gpio", &proplen);
+	if (prop) {
+		if (dp83640->slave) {
+			pr_err("dp83640,calibrate-gpio property can not be set together with dp83640,slave");
+			return -EINVAL;
+		}
+		clock->calibrate_gpio = -1;
+		of_property_read_u32(node, "dp83640,calibrate-gpio",
+				     &clock->calibrate_gpio);
+	}
+
+	if (!dp83640->slave) {
+		if (clock->caps.n_per_out + clock->caps.n_ext_ts +
+		    (clock->calibrate_gpio != -1 ? 1 : 0) > N_EXT) {
+			pr_err("Too many gpios configured");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id dp83640_of_match_table[] = {
+	{ .compatible = "national,dp83640", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, dp83640_of_match_table);
+#else
+
+static inline int dp83640_probe_dt(struct device_node *node,
+				   struct dp83640_private *dp83640)
+{
+	return 0;
+}
+#endif
+
 static int dp83640_probe(struct phy_device *phydev)
 {
 	struct dp83640_clock *clock;
@@ -977,7 +1031,13 @@ static int dp83640_probe(struct phy_device *phydev)
 	if (!dp83640)
 		goto no_memory;
 
+	dp83640->clock = clock;
 	dp83640->phydev = phydev;
+
+	err = dp83640_probe_dt(phydev->dev.of_node, dp83640);
+	if (err)
+		return err;
+
 	INIT_WORK(&dp83640->ts_work, rx_timestamp_work);
 
 	INIT_LIST_HEAD(&dp83640->rxts);
@@ -991,9 +1051,7 @@ static int dp83640_probe(struct phy_device *phydev)
 	skb_queue_head_init(&dp83640->rx_queue);
 	skb_queue_head_init(&dp83640->tx_queue);
 
-	dp83640->clock = clock;
-
-	if (choose_this_phy(clock, phydev)) {
+	if (!clock->chosen && !dp83640->slave) {
 		clock->chosen = dp83640;
 		clock->ptp_clock = ptp_clock_register(&clock->caps, &phydev->dev);
 		if (IS_ERR(clock->ptp_clock)) {
@@ -1338,7 +1396,10 @@ static struct phy_driver dp83640_driver = {
 	.hwtstamp	= dp83640_hwtstamp,
 	.rxtstamp	= dp83640_rxtstamp,
 	.txtstamp	= dp83640_txtstamp,
-	.driver		= {.owner = THIS_MODULE,}
+	.driver		= {
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(dp83640_of_match_table),
+	}
 };
 
 static int __init dp83640_init(void)
-- 
1.8.5.3

^ permalink raw reply related

* Re: [PATCH 1/2] ARM: shmobile: r8a7791: add Ether DT support
From: Ben Dooks @ 2014-02-10 12:54 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: horms, linux-sh, devicetree, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, magnus.damm, linux, linux-arm-kernel
In-Reply-To: <201402070112.48304.sergei.shtylyov@cogentembedded.com>

On 06/02/14 22:12, Sergei Shtylyov wrote:
> Define the generic R8A7791 part of the Ether device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
>   arch/arm/boot/dts/r8a7791.dtsi |   15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> Index: renesas/arch/arm/boot/dts/r8a7791.dtsi
> ===================================================================
> --- renesas.orig/arch/arm/boot/dts/r8a7791.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7791.dtsi
> @@ -2,7 +2,8 @@
>    * Device Tree Source for the r8a7791 SoC
>    *
>    * Copyright (C) 2013 Renesas Electronics Corporation
> - * Copyright (C) 2013 Renesas Solutions Corp.
> + * Copyright (C) 2013-2014 Renesas Solutions Corp.
> + * Copyright (C) 2014 Cogent Embedded Inc.
>    *
>    * This file is licensed under the terms of the GNU General Public License
>    * version 2.  This program is licensed "as is" without any warranty of any
> @@ -366,6 +367,18 @@
>   		status = "disabled";
>   	};
>
> +	ether: ethernet@ee700000 {
> +		compatible = "renesas,ether-r8a7791";
> +		reg = <0 0xee700000 0 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 162 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp8_clks R8A7791_CLK_ETHER>;
> +		phy-mode = "rmii";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +	};

Is the device only capable of doing RMII?


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

^ permalink raw reply

* Re: [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-02-10 12:51 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Srikanth Thokala, Vinod Koul, dan.j.williams, michal.simek,
	Grant Likely, robh+dt, linux-arm-kernel,
	linux-kernel@vger.kernel.org, devicetree, dmaengine
In-Reply-To: <52F3B017.5030100@metafoo.de>

On Thu, Feb 6, 2014 at 9:23 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/06/2014 02:34 PM, Srikanth Thokala wrote:
>>
>> On Wed, Feb 5, 2014 at 10:00 PM, Lars-Peter Clausen <lars@metafoo.de>
>> wrote:
>>>
>>> On 02/05/2014 05:25 PM, Srikanth Thokala wrote:
>>>>
>>>>
>>>> On Fri, Jan 31, 2014 at 12:21 PM, Srikanth Thokala <sthokal@xilinx.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi Vinod,
>>>>>
>>>>> On Tue, Jan 28, 2014 at 8:43 AM, Vinod Koul <vinod.koul@intel.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, Jan 27, 2014 at 06:42:36PM +0530, Srikanth Thokala wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Lars/Vinod,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The question here i think would be waht this device supports? Is
>>>>>>>>> the
>>>>>>>>> hardware
>>>>>>>>> capable of doing interleaved transfers, then would make sense.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The hardware does 2D transfers. The parameters for a transfer are
>>>>>>>> height,
>>>>>>>> width and stride. That's only a subset of what interleaved transfers
>>>>>>>> can be
>>>>>>>> (xt->num_frames must be one for 2d transfers). But if I remember
>>>>>>>> correctly
>>>>>>>> there has been some discussion on this in the past and the result of
>>>>>>>> that
>>>>>>>> discussion was that using interleaved transfers for 2D transfers is
>>>>>>>> preferred over adding a custom API for 2D transfers.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I went through the prep_interleaved_dma API and I see only one
>>>>>>> descriptor
>>>>>>> is prepared per API call (i.e. per frame).  As our IP supports upto
>>>>>>> 16
>>>>>>> frame
>>>>>>> buffers (can be more in future), isn't it less efficient compared to
>>>>>>> the
>>>>>>> prep_slave_sg where we get a single sg list and can prepare all the
>>>>>>> descriptors
>>>>>>> (of non-contiguous buffers) in one go?  Correct me, if am wrong and
>>>>>>> let
>>>>>>> me
>>>>>>> know your opinions.
>>>>>>
>>>>>>
>>>>>> Well the descriptor maybe one, but that can represent multiple frames,
>>>>>> for
>>>>>> example 16 as in your case. Can you read up the documentation of how
>>>>>> multiple
>>>>>> frames are passed. Pls see include/linux/dmaengine.h
>>>>>>
>>>>>> /**
>>>>>>    * Interleaved Transfer Request
>>>>>>    * ----------------------------
>>>>>>    * A chunk is collection of contiguous bytes to be transfered.
>>>>>>    * The gap(in bytes) between two chunks is called
>>>>>> inter-chunk-gap(ICG).
>>>>>>    * ICGs may or maynot change between chunks.
>>>>>>    * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
>>>>>>    *  that when repeated an integral number of times, specifies the
>>>>>> transfer.
>>>>>>    * A transfer template is specification of a Frame, the number of
>>>>>> times
>>>>>>    *  it is to be repeated and other per-transfer attributes.
>>>>>>    *
>>>>>>    * Practically, a client driver would have ready a template for each
>>>>>>    *  type of transfer it is going to need during its lifetime and
>>>>>>    *  set only 'src_start' and 'dst_start' before submitting the
>>>>>> requests.
>>>>>>    *
>>>>>>    *
>>>>>>    *  |      Frame-1        |       Frame-2       | ~ |
>>>>>> Frame-'numf'  |
>>>>>>    *  |====....==.===...=...|====....==.===...=...| ~
>>>>>> |====....==.===...=...|
>>>>>>    *
>>>>>>    *    ==  Chunk size
>>>>>>    *    ... ICG
>>>>>>    */
>>>>>
>>>>>
>>>>>
>>>>> Yes, it can handle multiple frames specified by 'numf' each of size
>>>>> 'frame_size * sgl[0].size'.
>>>>> But, I see it only works if all the frames' memory is contiguous and
>>>>> in this case we
>>>>> can just increment 'src_start' by the total frame size 'numf' number
>>>>> of times to fill in
>>>>> for each HW descriptor (each frame is one HW descriptor).  So, there
>>>>> is no issue when the
>>>>> memory is contiguous.  If the frames are non contiguous, we have to
>>>>> call this API for each
>>>>> frame (hence for each descriptor), as the src_start for each frame is
>>>>> different.  Is it correct?
>>>>>
>>>>> FYI: This hardware has an inbuilt Scatter-Gather engine.
>>>>>
>>>>
>>>> Ping?
>>>
>>>
>>>
>>> If you want to submit multiple frames at once I think you should look at
>>> how
>>> the current dmaengine API can be extended to allow that. And also provide
>>> an
>>> explanation on how this is superior over submitting them one by one.
>>
>>
>>
>> Sure.  I would start with explaning the current implementation of this
>> driver.
>>
>> Using prep_slave_sg(), we can define multiple segments in a
>> async_tx_descriptor where each frame is defined by a segment (a sg
>> list entry).  So, the slave device could DMA the data (of multiple
>> frames) with a descriptor by calling tx_submit in a transaction i.e.,
>>
>> prep_slave_sg(16)  -> tx_submit(1) -> interrupt  (16 frames)
>>
>> Using interleaved_dma(), we could not divide into segments when we
>> have scattered memory (for the reasons mentioned in above thread).
>> This implies we are restricting the slave device to process frame by
>> frame i.e.,
>>
>> interleaved_dma(1) -> tx_submit(1) -> interrupt -> interleaved_dma(2)
>> -> tx_submit (2) -> interrupt -> ........ tx_submit(16) -> interrupt
>>
>
> The API allows you to create and submit multiple interleaved descriptors
> before you have to issue them.
>
> interleaved_dma(1) -> tx_submit(1) -> interleaved_dma(2) -> tx_submit(2) ->
> ... -> issue_pending() -> interrupt
>
>
>> This implementation makes the hardware to wait until the next frame is
>> submitted.
>>
>> To overcome this, I feel it would be a good option if we could extend
>> interleaved_dma template to modify src_start/dest_start to be a
>> pointer to an array of addresses.  Here, number of addresses will be
>> defined by numf. The other option would be to include scatterlist in
>> the interleaved template. This way we can handle scattered memory
>> using this API.
>
>
> Each "frame" in a interleaved transfer describes a single line in your video
> frame (size = width, icg = stride). numf is the number of lines per video
> frame. So the suggested change does not make that much sense. If you want to
> submit multiple video frames in one batch the best option is in my opinion
> to allow to pass an array of dma_interleaved_template structs instead of a
> single one.

Ok.  I agree with you.  Will fix it in v3.

Srikanth

>
> - Lars
>
>>
>> Srikanth
>>
>>>
>>> - Lars
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>>> in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>>
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH v2 4/4] video: mmp: add device tree support
From: Tomi Valkeinen @ 2014-02-10 12:43 UTC (permalink / raw)
  To: Zhou Zhu, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	Jean-Christophe Plagniol-Villard, Haojian Zhuang, Sascha Hauer,
	Jingoo Han, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Chao Xie, Guoqing Li
In-Reply-To: <1389698184-28761-5-git-send-email-zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3657 bytes --]

On 14/01/14 13:16, Zhou Zhu wrote:
> add device tree support for mmp fb/controller
> the description of DT config is at
> Documentation/devicetree/bindings/fb/mmp-disp.txt
> 
> Signed-off-by: Zhou Zhu <zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/fb/mmp-disp.txt |   60 ++++++++
>  drivers/video/mmp/fb/mmpfb.c                      |   73 ++++++----
>  drivers/video/mmp/hw/mmp_ctrl.c                   |  160 ++++++++++++++++-----
>  3 files changed, 235 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
> 
> diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> new file mode 100644
> index 0000000..80702f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> @@ -0,0 +1,60 @@
> +* Marvell MMP Display (MMP_DISP)
> +
> +To config mmp display, 3 parts are required to be set in dts:
> +1. mmp fb
> +Required properties:
> +- compatible: Should be "marvell,<soc>-fb".
> +- marvell,path: Should be the path this fb connecting to.
> +- marvell,overlay-id: Should be the id of overlay this fb is on.
> +- marvell,dmafetch-id: Should be the dma fetch id this fb using.
> +- marvell,default-pixfmt: Should be the default pixel format when this fb is
> +turned on.
> +
> +2. mmp controller
> +Required properties:
> +- compatible: Should be "marvell,<soc>-disp".
> +- reg: Should be address and length of the register set for this controller.
> +- interrupts: Should be interrupt of this controller.
> +
> +Required sub-node:
> +- path:
> +Required properties in this sub-node:
> +-- marvell,overlay_num: Should be number of overlay this path has.

If that one tells how many overlays there are, maybe "num_overlays"
would be better.

> +-- marvell,output-type: Should be output-type settings
> +-- marvell,path-config: Should be path-config settings
> +-- marvell,link-config: Should be link-config settings
> +-- marvell,rbswap: Should be rbswap settings

If these terms (output-type, path-config, ...) are straight from the HW
manual, then fine. But if they are not clear, or are driver specific,
the values these can have should be documented here.

> +
> +3. panel
> +Required properties:
> +- marvell,path: Should be path that this panel connected to.
> +- other properties each panel has.
> +
> +Examples:
> +
> +fb: mmp-fb {
> +	compatible = "marvell,pxa988-fb";
> +	marvell,path = <&path1>;
> +	marvell,overlay-id = <0>;
> +	marvell,dmafetch-id = <1>;
> +	marvell,default-pixfmt = <0x108>;
> +};
> +
> +disp: mmp-disp@d420b000 {
> +	compatible = "marvell,pxa988-disp";
> +	reg = <0xd420b000 0x1fc>;
> +	interrupts = <0 41 0x4>;
> +	path1: mmp-pnpath {
> +		marvell,overlay-num = <2>;
> +		marvell,output-type = <0>;
> +		marvell,path-config = <0x20000000>;
> +		marvell,link-config = <0x60000001>;
> +		marvell,rbswap = <0>;
> +	};
> +};
> +
> +panel: <panel-name> {

How is the panel linked to all this? The nodes above do not refer to the
panel.

> +	...
> +	marvell,path = <&path1>;
> +	...
> +};

It's a bit difficult to say much about this, as I have no knowledge
about mmp.

But I don't quite understand what the pxa988-fb is. Is that some kind of
virtual device, only used to set up fbdev side? And pxa988-disp is the
actual hardware device?

If so, I don't really think pxa988-fb should exist in the DT data at
all, as it's not a hardware device.

Why isn't there just one node for pxa988-disp, which would contain all
the above information?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/4] video: mmp: add devm_mmp_get_path_by_phandle for DT
From: Tomi Valkeinen @ 2014-02-10 12:32 UTC (permalink / raw)
  To: Zhou Zhu, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	Jean-Christophe Plagniol-Villard, Haojian Zhuang, Sascha Hauer,
	Jingoo Han, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Chao Xie, Guoqing Li
In-Reply-To: <1389698184-28761-4-git-send-email-zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]

On 14/01/14 13:16, Zhou Zhu wrote:
> add devm_mmp_get_path_by_phandle to replace mmp_get_path
> for DT usage
> 
> Signed-off-by: Zhou Zhu <zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/video/mmp/core.c |   35 +++++++++++++++++++++++++++++++++++
>  include/video/mmp_disp.h |    2 ++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/video/mmp/core.c b/drivers/video/mmp/core.c
> index b563b92..0538458 100644
> --- a/drivers/video/mmp/core.c
> +++ b/drivers/video/mmp/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/slab.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/export.h>
> +#include <linux/of.h>
>  #include <video/mmp_disp.h>
>  
>  static struct mmp_overlay *path_get_overlay(struct mmp_path *path,
> @@ -156,6 +157,40 @@ struct mmp_path *mmp_get_path(const char *name)
>  EXPORT_SYMBOL_GPL(mmp_get_path);
>  
>  /*
> + * devm_mmp_get_path_by_phandle - get path by phandle
> + * @dev: device that want to get path
> + * @phandle: name of phandle in device node that want to get path
> + *
> + * this function gets path according to node pointed by phandle
> + * return NULL if no matching path
> + */
> +struct mmp_path *devm_mmp_get_path_by_phandle(struct device *dev,
> +	const char *phandle)
> +{
> +	struct mmp_path *path;
> +	struct device_node *node;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev, "device does not have a device node entry\n");
> +		return NULL;
> +	}
> +
> +	node = of_parse_phandle(dev->of_node, phandle, 0);
> +	if (!node) {
> +		dev_err(dev, "failed to get %s phandle in %s node\n", phandle,
> +			dev->of_node->name);
> +		return NULL;
> +	}
> +
> +	path = mmp_get_path(node->name);
> +
> +	of_node_put(node);
> +
> +	return path;
> +}
> +EXPORT_SYMBOL_GPL(devm_mmp_get_path_by_phandle);

Why is this function "devm_"? devm_ functions are supposed to
automatically free resources when the device is removed, I don't see
anything like that here.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* [PATCH RFC v2 19/35] imx-drm: convert to componentised device support
From: Russell King @ 2014-02-10 12:30 UTC (permalink / raw)
  To: Sascha Hauer, Shawn Guo, Fabio Estevam
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Greg Kroah-Hartman, Grant Likely,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
In-Reply-To: <20140210122802.GS26684-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Use the componentised device support for imx-drm.  This requires all
the sub-components and the master device to register with the component
device support.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/boot/dts/imx51-babbage.dts        |  10 ++-
 arch/arm/boot/dts/imx53-m53evk.dts         |   8 ++-
 arch/arm/boot/dts/imx53-mba53.dts          |   6 ++
 arch/arm/boot/dts/imx53-qsb.dts            |   8 ++-
 arch/arm/boot/dts/imx6q-sabresd.dts        |   4 ++
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi     |   6 ++
 drivers/staging/imx-drm/imx-drm-core.c     | 105 +++++++++++++++++++++++------
 drivers/staging/imx-drm/imx-ldb.c          |  40 +++++++----
 drivers/staging/imx-drm/imx-tve.c          |  63 ++++++++++-------
 drivers/staging/imx-drm/ipuv3-crtc.c       |  46 +++++++++----
 drivers/staging/imx-drm/parallel-display.c |  30 +++++++--
 11 files changed, 246 insertions(+), 80 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index be1407cf5abd..6ff15a0eacb3 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -21,7 +21,7 @@
 		reg = <0x90000000 0x20000000>;
 	};
 
-	display@di0 {
+	display0: display@di0 {
 		compatible = "fsl,imx-parallel-display";
 		crtcs = <&ipu 0>;
 		interface-pix-fmt = "rgb24";
@@ -43,7 +43,7 @@
 		};
 	};
 
-	display@di1 {
+	display1: display@di1 {
 		compatible = "fsl,imx-parallel-display";
 		crtcs = <&ipu 1>;
 		interface-pix-fmt = "rgb565";
@@ -81,6 +81,12 @@
 		};
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		crtcs = <&ipu 0>, <&ipu 1>;
+		connectors = <&display0>, <&display1>;
+	};
+
 	sound {
 		compatible = "fsl,imx51-babbage-sgtl5000",
 			     "fsl,imx-audio-sgtl5000";
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 7d304d02ed38..ee6107b6484c 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -21,7 +21,7 @@
 	};
 
 	soc {
-		display@di1 {
+		display1: display@di1 {
 			compatible = "fsl,imx-parallel-display";
 			crtcs = <&ipu 1>;
 			interface-pix-fmt = "bgr666";
@@ -53,6 +53,12 @@
 		default-brightness-level = <6>;
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		crtcs = <&ipu 1>;
+		connectors = <&display1>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 		pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/imx53-mba53.dts b/arch/arm/boot/dts/imx53-mba53.dts
index a63090267941..9b6e76980a74 100644
--- a/arch/arm/boot/dts/imx53-mba53.dts
+++ b/arch/arm/boot/dts/imx53-mba53.dts
@@ -43,6 +43,12 @@
 		status = "disabled";
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		crtcs = <&ipu 1>;
+		connectors = <&disp1>, <&tve>;
+	};
+
 	reg_3p2v: 3p2v {
 		compatible = "regulator-fixed";
 		regulator-name = "3P2V";
diff --git a/arch/arm/boot/dts/imx53-qsb.dts b/arch/arm/boot/dts/imx53-qsb.dts
index 91a5935a4aac..3cb4f7791a91 100644
--- a/arch/arm/boot/dts/imx53-qsb.dts
+++ b/arch/arm/boot/dts/imx53-qsb.dts
@@ -21,7 +21,7 @@
 		reg = <0x70000000 0x40000000>;
 	};
 
-	display@di0 {
+	display0: display@di0 {
 		compatible = "fsl,imx-parallel-display";
 		crtcs = <&ipu 0>;
 		interface-pix-fmt = "rgb565";
@@ -72,6 +72,12 @@
 		};
 	};
 
+	imx-drm {
+		compatible = "fsl,imx-drm";
+		crtcs = <&ipu 0>;
+		connectors = <&display0>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 		pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/imx6q-sabresd.dts b/arch/arm/boot/dts/imx6q-sabresd.dts
index 9cbdfe7a0931..66f220a82e45 100644
--- a/arch/arm/boot/dts/imx6q-sabresd.dts
+++ b/arch/arm/boot/dts/imx6q-sabresd.dts
@@ -20,6 +20,10 @@
 	compatible = "fsl,imx6q-sabresd", "fsl,imx6q";
 };
 
+&imx_drm {
+	crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+};
+
 &sata {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index e75e11b36dff..dfca3e001398 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -62,6 +62,12 @@
 		};
 	};
 
+	imx_drm: imx-drm {
+		compatible = "fsl,imx-drm";
+		crtcs = <&ipu1 0>, <&ipu1 1>;
+		connectors = <&ldb>;
+	};
+
 	sound {
 		compatible = "fsl,imx6q-sabresd-wm8962",
 			   "fsl,imx-audio-wm8962";
diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index d9786eca3a36..82b0337096b0 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -13,7 +13,7 @@
  * GNU General Public License for more details.
  *
  */
-
+#include <linux/component.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <drm/drmP.h>
@@ -90,6 +90,8 @@ static int imx_drm_driver_unload(struct drm_device *drm)
 {
 	struct imx_drm_device *imxdrm = drm->dev_private;
 
+	component_unbind_all(drm->dev, drm);
+
 	imx_drm_device_put();
 
 	drm_vblank_cleanup(drm);
@@ -371,11 +373,8 @@ static void imx_drm_connector_unregister(
 }
 
 /*
- * Called by the CRTC driver when all CRTCs are registered. This
- * puts all the pieces together and initializes the driver.
- * Once this is called no more CRTCs can be registered since
- * the drm core has hardcoded the number of crtcs in several
- * places.
+ * Main DRM initialisation. This binds, initialises and registers
+ * with DRM the subcomponents of the driver.
  */
 static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 {
@@ -428,8 +427,15 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
 
 	platform_set_drvdata(drm->platformdev, drm);
 	mutex_unlock(&imxdrm->mutex);
+
+	/* Now try and bind all our sub-components */
+	ret = component_bind_all(drm->dev, drm);
+	if (ret)
+		goto err_relock;
 	return 0;
 
+err_relock:
+	mutex_lock(&imxdrm->mutex);
 err_vblank:
 	drm_vblank_cleanup(drm);
 err_kms:
@@ -809,6 +815,70 @@ static struct drm_driver imx_drm_driver = {
 	.patchlevel		= 0,
 };
 
+static int compare_parent_of(struct device *dev, void *data)
+{
+	struct of_phandle_args *args = data;
+	return dev->parent && dev->parent->of_node == args->np;
+}
+
+static int compare_of(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+static int imx_drm_add_components(struct device *master, struct master *m)
+{
+	struct device_node *np = master->of_node;
+	unsigned i;
+	int ret;
+
+	for (i = 0; ; i++) {
+		struct of_phandle_args args;
+
+		ret = of_parse_phandle_with_fixed_args(np, "crtcs", 1,
+						       i, &args);
+		if (ret)
+			break;
+
+		ret = component_master_add_child(m, compare_parent_of, &args);
+		of_node_put(args.np);
+
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; ; i++) {
+		struct device_node *node;
+
+		node = of_parse_phandle(np, "connectors", i);
+		if (!node)
+			break;
+
+		ret = component_master_add_child(m, compare_of, node);
+		of_node_put(node);
+
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int imx_drm_bind(struct device *dev)
+{
+	return drm_platform_init(&imx_drm_driver, to_platform_device(dev));
+}
+
+static void imx_drm_unbind(struct device *dev)
+{
+	drm_put_dev(dev_get_drvdata(dev));
+}
+
+static const struct component_master_ops imx_drm_ops = {
+	.add_components = imx_drm_add_components,
+	.bind = imx_drm_bind,
+	.unbind = imx_drm_unbind,
+};
+
 static int imx_drm_platform_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -819,27 +889,31 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 
 	imx_drm_device->dev = &pdev->dev;
 
-	return drm_platform_init(&imx_drm_driver, pdev);
+	return component_master_add(&pdev->dev, &imx_drm_ops);
 }
 
 static int imx_drm_platform_remove(struct platform_device *pdev)
 {
-	drm_put_dev(platform_get_drvdata(pdev));
-
+	component_master_del(&pdev->dev, &imx_drm_ops);
 	return 0;
 }
 
+static const struct of_device_id imx_drm_dt_ids[] = {
+	{ .compatible = "fsl,imx-drm", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, imx_drm_dt_ids);
+
 static struct platform_driver imx_drm_pdrv = {
 	.probe		= imx_drm_platform_probe,
 	.remove		= imx_drm_platform_remove,
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "imx-drm",
+		.of_match_table = imx_drm_dt_ids,
 	},
 };
 
-static struct platform_device *imx_drm_pdev;
-
 static int __init imx_drm_init(void)
 {
 	int ret;
@@ -852,12 +926,6 @@ static int __init imx_drm_init(void)
 	INIT_LIST_HEAD(&imx_drm_device->connector_list);
 	INIT_LIST_HEAD(&imx_drm_device->encoder_list);
 
-	imx_drm_pdev = platform_device_register_simple("imx-drm", -1, NULL, 0);
-	if (IS_ERR(imx_drm_pdev)) {
-		ret = PTR_ERR(imx_drm_pdev);
-		goto err_pdev;
-	}
-
 	ret = platform_driver_register(&imx_drm_pdrv);
 	if (ret)
 		goto err_pdrv;
@@ -865,8 +933,6 @@ static int __init imx_drm_init(void)
 	return 0;
 
 err_pdrv:
-	platform_device_unregister(imx_drm_pdev);
-err_pdev:
 	kfree(imx_drm_device);
 
 	return ret;
@@ -874,7 +940,6 @@ static int __init imx_drm_init(void)
 
 static void __exit imx_drm_exit(void)
 {
-	platform_device_unregister(imx_drm_pdev);
 	platform_driver_unregister(&imx_drm_pdrv);
 
 	kfree(imx_drm_device);
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
index dd29a4aad376..d00f93f3440d 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -20,6 +20,7 @@
 
 #include <linux/module.h>
 #include <linux/clk.h>
+#include <linux/component.h>
 #include <drm/drmP.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
@@ -450,11 +451,11 @@ static const struct of_device_id imx_ldb_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, imx_ldb_dt_ids);
 
-static int imx_ldb_probe(struct platform_device *pdev)
+static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = dev->of_node;
 	const struct of_device_id *of_id =
-			of_match_device(imx_ldb_dt_ids, &pdev->dev);
+			of_match_device(imx_ldb_dt_ids, dev);
 	struct device_node *child;
 	const u8 *edidp;
 	struct imx_ldb *imx_ldb;
@@ -464,17 +465,17 @@ static int imx_ldb_probe(struct platform_device *pdev)
 	int ret;
 	int i;
 
-	imx_ldb = devm_kzalloc(&pdev->dev, sizeof(*imx_ldb), GFP_KERNEL);
+	imx_ldb = devm_kzalloc(dev, sizeof(*imx_ldb), GFP_KERNEL);
 	if (!imx_ldb)
 		return -ENOMEM;
 
 	imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
 	if (IS_ERR(imx_ldb->regmap)) {
-		dev_err(&pdev->dev, "failed to get parent regmap\n");
+		dev_err(dev, "failed to get parent regmap\n");
 		return PTR_ERR(imx_ldb->regmap);
 	}
 
-	imx_ldb->dev = &pdev->dev;
+	imx_ldb->dev = dev;
 
 	if (of_id)
 		imx_ldb->lvds_mux = of_id->data;
@@ -512,7 +513,7 @@ static int imx_ldb_probe(struct platform_device *pdev)
 			return -EINVAL;
 
 		if (dual && i > 0) {
-			dev_warn(&pdev->dev, "dual-channel mode, ignoring second output\n");
+			dev_warn(dev, "dual-channel mode, ignoring second output\n");
 			continue;
 		}
 
@@ -551,7 +552,7 @@ static int imx_ldb_probe(struct platform_device *pdev)
 			break;
 		case LVDS_BIT_MAP_JEIDA:
 			if (datawidth == 18) {
-				dev_err(&pdev->dev, "JEIDA standard only supported in 24 bit\n");
+				dev_err(dev, "JEIDA standard only supported in 24 bit\n");
 				return -EINVAL;
 			}
 			if (i == 0 || dual)
@@ -560,7 +561,7 @@ static int imx_ldb_probe(struct platform_device *pdev)
 				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 | LDB_BIT_MAP_CH1_JEIDA;
 			break;
 		default:
-			dev_err(&pdev->dev, "data mapping not specified or invalid\n");
+			dev_err(dev, "data mapping not specified or invalid\n");
 			return -EINVAL;
 		}
 
@@ -571,14 +572,15 @@ static int imx_ldb_probe(struct platform_device *pdev)
 		imx_drm_encoder_add_possible_crtcs(channel->imx_drm_encoder, child);
 	}
 
-	platform_set_drvdata(pdev, imx_ldb);
+	dev_set_drvdata(dev, imx_ldb);
 
 	return 0;
 }
 
-static int imx_ldb_remove(struct platform_device *pdev)
+static void imx_ldb_unbind(struct device *dev, struct device *master,
+	void *data)
 {
-	struct imx_ldb *imx_ldb = platform_get_drvdata(pdev);
+	struct imx_ldb *imx_ldb = dev_get_drvdata(dev);
 	int i;
 
 	for (i = 0; i < 2; i++) {
@@ -591,7 +593,21 @@ static int imx_ldb_remove(struct platform_device *pdev)
 		imx_drm_remove_connector(channel->imx_drm_connector);
 		imx_drm_remove_encoder(channel->imx_drm_encoder);
 	}
+}
 
+static const struct component_ops imx_ldb_ops = {
+	.bind	= imx_ldb_bind,
+	.unbind	= imx_ldb_unbind,
+};
+
+static int imx_ldb_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &imx_ldb_ops);
+}
+
+static int imx_ldb_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &imx_ldb_ops);
 	return 0;
 }
 
diff --git a/drivers/staging/imx-drm/imx-tve.c b/drivers/staging/imx-drm/imx-tve.c
index 77131e5770f3..ad840d78a09a 100644
--- a/drivers/staging/imx-drm/imx-tve.c
+++ b/drivers/staging/imx-drm/imx-tve.c
@@ -20,6 +20,7 @@
 
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/component.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/regmap.h>
@@ -583,9 +584,10 @@ static const int of_get_tve_mode(struct device_node *np)
 	return -EINVAL;
 }
 
-static int imx_tve_probe(struct platform_device *pdev)
+static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *np = dev->of_node;
 	struct device_node *ddc_node;
 	struct imx_tve *tve;
 	struct resource *res;
@@ -594,11 +596,11 @@ static int imx_tve_probe(struct platform_device *pdev)
 	int irq;
 	int ret;
 
-	tve = devm_kzalloc(&pdev->dev, sizeof(*tve), GFP_KERNEL);
+	tve = devm_kzalloc(dev, sizeof(*tve), GFP_KERNEL);
 	if (!tve)
 		return -ENOMEM;
 
-	tve->dev = &pdev->dev;
+	tve->dev = dev;
 	spin_lock_init(&tve->lock);
 
 	ddc_node = of_parse_phandle(np, "ddc", 0);
@@ -609,7 +611,7 @@ static int imx_tve_probe(struct platform_device *pdev)
 
 	tve->mode = of_get_tve_mode(np);
 	if (tve->mode != TVE_MODE_VGA) {
-		dev_err(&pdev->dev, "only VGA mode supported, currently\n");
+		dev_err(dev, "only VGA mode supported, currently\n");
 		return -EINVAL;
 	}
 
@@ -618,7 +620,7 @@ static int imx_tve_probe(struct platform_device *pdev)
 					   &tve->hsync_pin);
 
 		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to get vsync pin\n");
+			dev_err(dev, "failed to get vsync pin\n");
 			return ret;
 		}
 
@@ -626,40 +628,40 @@ static int imx_tve_probe(struct platform_device *pdev)
 					    &tve->vsync_pin);
 
 		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to get vsync pin\n");
+			dev_err(dev, "failed to get vsync pin\n");
 			return ret;
 		}
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
+	base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
 	tve_regmap_config.lock_arg = tve;
-	tve->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "tve", base,
+	tve->regmap = devm_regmap_init_mmio_clk(dev, "tve", base,
 						&tve_regmap_config);
 	if (IS_ERR(tve->regmap)) {
-		dev_err(&pdev->dev, "failed to init regmap: %ld\n",
+		dev_err(dev, "failed to init regmap: %ld\n",
 			PTR_ERR(tve->regmap));
 		return PTR_ERR(tve->regmap);
 	}
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
-		dev_err(&pdev->dev, "failed to get irq\n");
+		dev_err(dev, "failed to get irq\n");
 		return irq;
 	}
 
-	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+	ret = devm_request_threaded_irq(dev, irq, NULL,
 					imx_tve_irq_handler, IRQF_ONESHOT,
 					"imx-tve", tve);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to request irq: %d\n", ret);
+		dev_err(dev, "failed to request irq: %d\n", ret);
 		return ret;
 	}
 
-	tve->dac_reg = devm_regulator_get(&pdev->dev, "dac");
+	tve->dac_reg = devm_regulator_get(dev, "dac");
 	if (!IS_ERR(tve->dac_reg)) {
 		regulator_set_voltage(tve->dac_reg, 2750000, 2750000);
 		ret = regulator_enable(tve->dac_reg);
@@ -667,17 +669,17 @@ static int imx_tve_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	tve->clk = devm_clk_get(&pdev->dev, "tve");
+	tve->clk = devm_clk_get(dev, "tve");
 	if (IS_ERR(tve->clk)) {
-		dev_err(&pdev->dev, "failed to get high speed tve clock: %ld\n",
+		dev_err(dev, "failed to get high speed tve clock: %ld\n",
 			PTR_ERR(tve->clk));
 		return PTR_ERR(tve->clk);
 	}
 
 	/* this is the IPU DI clock input selector, can be parented to tve_di */
-	tve->di_sel_clk = devm_clk_get(&pdev->dev, "di_sel");
+	tve->di_sel_clk = devm_clk_get(dev, "di_sel");
 	if (IS_ERR(tve->di_sel_clk)) {
-		dev_err(&pdev->dev, "failed to get ipu di mux clock: %ld\n",
+		dev_err(dev, "failed to get ipu di mux clock: %ld\n",
 			PTR_ERR(tve->di_sel_clk));
 		return PTR_ERR(tve->di_sel_clk);
 	}
@@ -688,11 +690,11 @@ static int imx_tve_probe(struct platform_device *pdev)
 
 	ret = regmap_read(tve->regmap, TVE_COM_CONF_REG, &val);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to read configuration register: %d\n", ret);
+		dev_err(dev, "failed to read configuration register: %d\n", ret);
 		return ret;
 	}
 	if (val != 0x00100000) {
-		dev_err(&pdev->dev, "configuration register default value indicates this is not a TVEv2\n");
+		dev_err(dev, "configuration register default value indicates this is not a TVEv2\n");
 		return -ENODEV;
 	}
 
@@ -705,14 +707,15 @@ static int imx_tve_probe(struct platform_device *pdev)
 
 	ret = imx_drm_encoder_add_possible_crtcs(tve->imx_drm_encoder, np);
 
-	platform_set_drvdata(pdev, tve);
+	dev_set_drvdata(dev, tve);
 
 	return 0;
 }
 
-static int imx_tve_remove(struct platform_device *pdev)
+static void imx_tve_unbind(struct device *dev, struct device *master,
+	void *data)
 {
-	struct imx_tve *tve = platform_get_drvdata(pdev);
+	struct imx_tve *tve = dev_get_drvdata(dev);
 	struct drm_connector *connector = &tve->connector;
 	struct drm_encoder *encoder = &tve->encoder;
 
@@ -723,7 +726,21 @@ static int imx_tve_remove(struct platform_device *pdev)
 
 	if (!IS_ERR(tve->dac_reg))
 		regulator_disable(tve->dac_reg);
+}
 
+static const struct component_ops imx_tve_ops = {
+	.bind	= imx_tve_bind,
+	.unbind	= imx_tve_unbind,
+};
+
+static int imx_tve_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &imx_tve_ops);
+}
+
+static int imx_tve_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &imx_tve_ops);
 	return 0;
 }
 
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 08e0a3b29174..d779ad2d1f4e 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -17,6 +17,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
  * MA 02110-1301, USA.
  */
+#include <linux/component.h>
 #include <linux/module.h>
 #include <linux/export.h>
 #include <linux/device.h>
@@ -400,43 +401,60 @@ static int ipu_crtc_init(struct ipu_crtc *ipu_crtc,
 	return ret;
 }
 
-static int ipu_drm_probe(struct platform_device *pdev)
+static int ipu_drm_bind(struct device *dev, struct device *master, void *data)
 {
-	struct ipu_client_platformdata *pdata = pdev->dev.platform_data;
+	struct ipu_client_platformdata *pdata = dev->platform_data;
 	struct ipu_crtc *ipu_crtc;
 	int ret;
 
-	if (!pdata)
-		return -EINVAL;
-
-	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
-	if (ret)
-		return ret;
-
-	ipu_crtc = devm_kzalloc(&pdev->dev, sizeof(*ipu_crtc), GFP_KERNEL);
+	ipu_crtc = devm_kzalloc(dev, sizeof(*ipu_crtc), GFP_KERNEL);
 	if (!ipu_crtc)
 		return -ENOMEM;
 
-	ipu_crtc->dev = &pdev->dev;
+	ipu_crtc->dev = dev;
 
 	ret = ipu_crtc_init(ipu_crtc, pdata);
 	if (ret)
 		return ret;
 
-	platform_set_drvdata(pdev, ipu_crtc);
+	dev_set_drvdata(dev, ipu_crtc);
 
 	return 0;
 }
 
-static int ipu_drm_remove(struct platform_device *pdev)
+static void ipu_drm_unbind(struct device *dev, struct device *master,
+	void *data)
 {
-	struct ipu_crtc *ipu_crtc = platform_get_drvdata(pdev);
+	struct ipu_crtc *ipu_crtc = dev_get_drvdata(dev);
 
 	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
 
 	ipu_plane_put_resources(ipu_crtc->plane[0]);
 	ipu_put_resources(ipu_crtc);
+}
+
+static const struct component_ops ipu_crtc_ops = {
+	.bind = ipu_drm_bind,
+	.unbind = ipu_drm_unbind,
+};
 
+static int ipu_drm_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	if (!pdev->dev.platform_data)
+		return -EINVAL;
+
+	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
+	return component_add(&pdev->dev, &ipu_crtc_ops);
+}
+
+static int ipu_drm_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &ipu_crtc_ops);
 	return 0;
 }
 
diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c
index 12bcf4f58bdf..4019cae35ff9 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -18,6 +18,7 @@
  * MA 02110-1301, USA.
  */
 
+#include <linux/component.h>
 #include <linux/module.h>
 #include <drm/drmP.h>
 #include <drm/drm_fb_helper.h>
@@ -192,15 +193,15 @@ static int imx_pd_register(struct imx_parallel_display *imxpd)
 	return 0;
 }
 
-static int imx_pd_probe(struct platform_device *pdev)
+static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = dev->of_node;
 	const u8 *edidp;
 	struct imx_parallel_display *imxpd;
 	int ret;
 	const char *fmt;
 
-	imxpd = devm_kzalloc(&pdev->dev, sizeof(*imxpd), GFP_KERNEL);
+	imxpd = devm_kzalloc(dev, sizeof(*imxpd), GFP_KERNEL);
 	if (!imxpd)
 		return -ENOMEM;
 
@@ -218,7 +219,7 @@ static int imx_pd_probe(struct platform_device *pdev)
 			imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
 	}
 
-	imxpd->dev = &pdev->dev;
+	imxpd->dev = dev;
 
 	ret = imx_pd_register(imxpd);
 	if (ret)
@@ -226,14 +227,15 @@ static int imx_pd_probe(struct platform_device *pdev)
 
 	ret = imx_drm_encoder_add_possible_crtcs(imxpd->imx_drm_encoder, np);
 
-	platform_set_drvdata(pdev, imxpd);
+	dev_set_drvdata(dev, imxpd);
 
 	return 0;
 }
 
-static int imx_pd_remove(struct platform_device *pdev)
+static void imx_pd_unbind(struct device *dev, struct device *master,
+	void *data)
 {
-	struct imx_parallel_display *imxpd = platform_get_drvdata(pdev);
+	struct imx_parallel_display *imxpd = dev_get_drvdata(dev);
 	struct drm_connector *connector = &imxpd->connector;
 	struct drm_encoder *encoder = &imxpd->encoder;
 
@@ -241,7 +243,21 @@ static int imx_pd_remove(struct platform_device *pdev)
 
 	imx_drm_remove_connector(imxpd->imx_drm_connector);
 	imx_drm_remove_encoder(imxpd->imx_drm_encoder);
+}
 
+static const struct component_ops imx_pd_ops = {
+	.bind	= imx_pd_bind,
+	.unbind	= imx_pd_unbind,
+};
+
+static int imx_pd_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &imx_pd_ops);
+}
+
+static int imx_pd_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &imx_pd_ops);
 	return 0;
 }
 
-- 
1.8.3.1

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

^ permalink raw reply related

* [PATCH RFC v2 14/35] imx-drm: update and fix imx6 DT descriptions for v3 HDMI driver
From: Russell King @ 2014-02-10 12:29 UTC (permalink / raw)
  To: Sascha Hauer, Shawn Guo, Fabio Estevam
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140210122802.GS26684-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6dl.dtsi  | 3 ++-
 arch/arm/boot/dts/imx6q.dtsi   | 1 +
 arch/arm/boot/dts/imx6qdl.dtsi | 1 -
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 65e54b4529c5..6dc397022214 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -90,5 +90,6 @@
 };
 
 &hdmi {
+	compatible = "fsl,imx6dl-hdmi";
 	crtcs = <&ipu1 0>, <&ipu1 1>;
-}
+};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index d2467f532de6..187fe33ba515 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -161,5 +161,6 @@
 };
 
 &hdmi {
+	compatible = "fsl,imx6q-hdmi";
 	crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
 };
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 400bbc624f25..930ebe0c2937 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1369,7 +1369,6 @@
 			};
 
 			hdmi: hdmi@0120000 {
-				compatible = "fsl,imx6q-hdmi";
 				reg = <0x00120000 0x9000>;
 				interrupts = <0 115 0x04>;
 				gpr = <&gpr>;
-- 
1.8.3.1

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

^ permalink raw reply related

* [PATCH RFC v2 13/35] imx-drm: add imx6 DT configuration for HDMI
From: Russell King @ 2014-02-10 12:29 UTC (permalink / raw)
  To: Sascha Hauer, Shawn Guo, Fabio Estevam
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140210122802.GS26684-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Extracted from another patch by Fabio Estevam, this adds the DT
configuration for HDMI output on the IMX6 SoCs

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6dl.dtsi  |  4 ++++
 arch/arm/boot/dts/imx6q.dtsi   |  4 ++++
 arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 9e8ae118fdd4..65e54b4529c5 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -88,3 +88,7 @@
 		crtcs = <&ipu1 0>, <&ipu1 1>;
 	};
 };
+
+&hdmi {
+	crtcs = <&ipu1 0>, <&ipu1 1>;
+}
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f024ef28b34b..d2467f532de6 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -159,3 +159,7 @@
 		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
 	};
 };
+
+&hdmi {
+	crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+};
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index fb28b2ecb1db..400bbc624f25 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1368,6 +1368,16 @@
 				};
 			};
 
+			hdmi: hdmi@0120000 {
+				compatible = "fsl,imx6q-hdmi";
+				reg = <0x00120000 0x9000>;
+				interrupts = <0 115 0x04>;
+				gpr = <&gpr>;
+				clocks = <&clks 123>, <&clks 124>;
+				clock-names = "iahb", "isfr";
+				status = "disabled";
+			};
+
 			dcic1: dcic@020e4000 {
 				reg = <0x020e4000 0x4000>;
 				interrupts = <0 124 0x04>;
-- 
1.8.3.1

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

^ permalink raw reply related

* Re: [PATCH v6 05/19] watchdog: orion: Make sure the watchdog is initially stopped
From: Ezequiel Garcia @ 2014-02-10 12:22 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
	Jason Cooper, Thomas Petazzoni, Gregory Clement, Lior Amsalem,
	Sebastian Hesselbarth, Andrew Lunn
In-Reply-To: <20140207174314.GB16263-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Fri, Feb 07, 2014 at 10:43:14AM -0700, Jason Gunthorpe wrote:
> On Fri, Feb 07, 2014 at 07:40:45AM -0300, Ezequiel Garcia wrote:
> 
> > Well, this is related to the discussion about the bootloader not
> > reseting the watchdog properly, provoking spurious watchdog triggering.
> > 
> > Jason Gunthorpe explained [1] that we needed a particular sequence:
> > 
> >  1. Disable WDT
> >  2. Clear bridge
> >  3. Enable WDT
> > 
> > We added the irq handling to satisfy (2), and the watchdog stop for (1).
> 
> The issue here is the driver configures two 'machine kill' elements:
> the PANIC IRQ and the RstOut setup.
> 
> Before configuring either of those the driver needs to ensure that any
> old watchdog events are cleared out of the HW. We must not get a
> spurious event. 
> 
> I agree not disabling an already functional and properly configured
> counter from the bootloader is desirable.
> 
> So lets break it down a bit..
> 
> 1) The IRQ:
>   It looks like the cause bit latches high on watchdog timer
>   expiration but has no side effect unless it is unmasked.
> 
>   The new IRQ flow code ensures the bit is cleared during request_irq
>   so no old events can trigger the IRQ. Thus it is solved now.
> 

Agreed.

> 3) The timer itself:
>   The WDT is just a general timer with an optional hookup to the
>   rst control. If it is harmlessly counting but not resetting we need
>   to stop that before enabling rst out.
> 

Actually, the current flow is to:

1. Disable rst out and then disable the counter, in probe().

2. Enable the counter, and then enable rst out, in start().

> So, how about this for psuedo-code in probe:
> 
> if (readl(RSTOUTn) & WDRstOutEn)
> {
>     /* Watchdog is configured and may be down counting,
>        don't touch it */
>     request_irq(..);
> }
> else
> {
>     /* Watchdog is not configured, fully disable the timer
>        and configure for watchdog operation. */
>     disable_watchdog();
>     request_irq();
>     writel(RSTOUTn), .. WDRstOutEn);
> }
> 

Sounds good, although it seems to me it's actually simpler:

  /* Let's make sure the watchdog is fully stopped, unless
   * it's explicitly enabled and running
   */
  if ( !(wdt_rst_out_en && wdt_timer_enabled) ) {
    watchdog_stop();
  }

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCHv2 0/4] Add DT support for fixed PHYs
From: Thomas Petazzoni @ 2014-02-10 12:09 UTC (permalink / raw)
  To: Christian Gmeiner
  Cc: David S. Miller, netdev@vger.kernel.org, devicetree,
	Florian Fainelli, Lior Amsalem, Gregory Clement, Ezequiel Garcia,
	linux-arm-kernel@lists.infradead.org, Mark Rutland
In-Reply-To: <CAH9NwWfutp6MitSuG-LNH=0Y0ffewuSawLXJOe2JUL7=tz7BWQ@mail.gmail.com>

Dear Christian Gmeiner,

On Mon, 10 Feb 2014 11:30:30 +0100, Christian Gmeiner wrote:

> >> +1 for the general idea. This really looks good now. I've not much more
> >> to say. Maybe someone from the devicetree corner has a few words for the
> >> binding?
> >>
> >
> > I tested the whole series with an I.MX6D board with the FEC driver:
> > fec 2188000.ethernet eth0: Freescale FEC PHY driver [Generic PHY]
> > (mii_bus:phy_addr=fixed-0:00, irq=-1)
> >
> > For me binding looks nice and I hope to see this patch series in 3.13.
> >
> > Tested-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> 
> Is there any update on this patch series?

I'll try to send a new version in the next few days. It's still part of
my TODO list.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices
From: Lee Jones @ 2014-02-10 11:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: Adam Thomson, alsa-devel, linux-kernel, devicetree, Rob Herring,
	Linus Walleij, Dmitry Torokhov, Alessandro Zummo, Guenter Roeck
In-Reply-To: <20140207125831.GL32298@sirena.org.uk>

> > > +/*
> > > + * DO NOT change the device Ids. The naming is intentionally specific as both
> > > + * the PMIC and CODEC parts of this chip are instantiated separately as I2C
> > > + * devices (both have configurable I2C addresses, and are to all intents and
> > > + * purposes separate). As a result there are specific DA9055 ids for PMIC
> > > + * and CODEC, which must be different to operate together.
> > > + */
> 
> > I'm not sure this comment is required.
> 
> They are, we've already had the suffixes removed from both PMIC and
> CODEC drivers by people doing code review causing the drivers to fail to
> load for several kernel releases (this should be tagged to stable as a
> result).
> 
> > Most device IDs are named this way.
> 
> Having the suffix on a subdevice would be normal but it's not normal for
> the primary I2C device, usually you can just put the part number in.

Okay, no problem then. Ignore my last.

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

^ permalink raw reply

* [PATCH] dt/bindings: update fsl-fec regarding compatible and clocks
From: Shawn Guo @ 2014-02-10 11:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Philippe De Muyter, Shawn Guo

Update fsl-fec to explicitly list the supported compatible strings
and add missing 'clocks' and 'clock-names' properties.  It does not
change anything about how kernel drive works.  Instead, it just reflects
how kernel driver works today.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 845ff84..3ebd395 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -1,9 +1,26 @@
 * Freescale Fast Ethernet Controller (FEC)
 
 Required properties:
-- compatible : Should be "fsl,<soc>-fec"
+- compatible : Should contain one of the following:
+		"fsl,imx25-fec"
+		"fsl,imx27-fec"
+		"fsl,imx28-fec"
+		"fsl,imx6q-fec"
+		"fsl,mvf600-fec"
 - reg : Address and length of the register set for the device
 - interrupts : Should contain fec interrupt
+- clocks: phandle to the clocks feeding the FEC controller and phy. The
+  following two are required:
+   - "ipg": the peripheral access clock
+   - "ahb": the bus clock for MAC
+  The following two are optional:
+   - "ptp": the sampling clock for PTP (IEEE 1588).  On SoC like i.MX6Q,
+     the clock could come from either the internal clock control module
+     or external oscillator via pad depending on board design.
+   - "enet_out": the phy reference clock provided by SoC via pad, which
+     is available on SoC like i.MX28.
+- clock-names: Must contain the clock names described just above
+
 - phy-mode : String, operation mode of the PHY interface.
   Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
   "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
-- 
1.7.9.5


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

^ permalink raw reply related

* Re: [PATCH 4/4] ARM: Kirkwood: Add support for many Synology NAS devices
From: Ian Campbell @ 2014-02-10 11:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, linux ARM, Ben Peddell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	pawel.moll-5wv7dgnIgG8, Mark Rutland, Kumar Gala
In-Reply-To: <20140206160126.GH29860-g2DYL2Zd6BY@public.gmane.org>

On Thu, 2014-02-06 at 17:01 +0100, Andrew Lunn wrote:
> Synology seem to build there devices like lego. They have two
> different RTC blocks. They have three different fan alarm blocks, four
> different led blocks, etc. And to build a product, the just select a
> group of blocks and put them together.
> 
> The board setup code which Ben Peddell wrote has a somewhat similar
> structure:
> 
> http://klightspeed.killerwolves.net/synology/linux-3.4-synology-0.1.patch
> 
> It has a set of functions which add platform devices. And a table
> driven piece of code which based on the product name calls these
> functions to add the needed platform devices. Take a look at the table
> to get a better idea of the re-use factor of the blocks.
> 
> In this DT version, i have a dtsi file for each function, and a dti
> file for each table entry.

At least some other platforms deal with this by having a baseline dtsi
where most things have status="disabled" and then a per-board .dts file
which contains status="okay" and perhaps any specific pin bindings etc.

See arch/arm/boot/dts/sun?i* for an example of this approach.

Ian.


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

^ permalink raw reply

* Re: Devicetree Maintenance in barebox
From: Ian Campbell @ 2014-02-10 11:38 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Sascha Hauer, Grant Likely,
	barebox-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140207141028.GT8533-u4khhh1J0LxI1Ri9qeTfzeTW4wlIGRCZ@public.gmane.org>

On Fri, 2014-02-07 at 09:10 -0500, Jason Cooper wrote:
> Hi Sascha,
> 
> + Grant Likely, Ian Campbell, devicetree ML
> 
> This discussion started on the barebox bootloader mailinglist
> 
> On Fri, Feb 07, 2014 at 08:13:32AM +0100, Sascha Hauer wrote:
> > It's becoming more obvious that devicetree maintenance is painful
> > because we have to sync them to the kernel regularly. My hope was that
> > this would get simpler once the devicetrees get their own repository
> > outside the kernel, but it seems that won't happen anytime soon.
> 
> hmm.  Ian Campbell has a tree he is working on:
> 
>   git://xenbits.xen.org/people/ianc/device-tree-rebasing.git

This is automatically extracted out of the linux git tree (using
git-filter-branch), maintaining all the history etc. I intend to keep
this running until such a time as the DTS files are migrated out of
Linux.

For the time being though patches to this tree need to go through the
Linux tree in the normal way.

(also, as the name states it is potentially rebasing, although there
hasn't been an actual need since the early days when I was still getting
things working)

Ian.

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

^ permalink raw reply

* Re: Devicetree Maintenance in barebox
From: Ian Campbell @ 2014-02-10 11:35 UTC (permalink / raw)
  To: Jon Loeliger
  Cc: Grant Likely, barebox-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Jason Cooper, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <E1WCYdq-0004eu-Gr-CYoMK+44s/E@public.gmane.org>

On Sun, 2014-02-09 at 11:58 -0600, Jon Loeliger wrote:
> > Hi Sascha,
> > 
> > + Grant Likely, Ian Campbell, devicetree ML
> > 
> > Also, In the DT meeting earlier this week, Grant Likely said he has the
> > request in to create a separate mailinglist for collaboration between
> > the different devicetree users (BSD, Linux, etc).
> 
> ...
> 
> > I think the proper solution will percolate out of the first
> > cross-project discussions on the new ML.
> 
> ...
> 
> > Definitely fodder for the new ML.
> > 
> > Grant, can you please add Sascha to the list of folks to notify when
> > the new ML is ready?
> 
> I don't think there needs to be a different mailing list
> in order to combine or discuss other OS's use of the device
> tree compiler.  The  DTC is OS and Use-agnostic.  Discussions
> of DTC needs for FreeBSD can happen right here as the orginal
> purpose of this list was DTC discussion.
> 
> Are you, and Grant(?), suggesting that a separate list
> should be created for FreeBSD use of DTS-file contents?
> Or that DTS-file-content related discussions should be
> separated from DTC discussions?

The latter. See http://www.spinics.net/lists/devicetree/msg19209.html .
The issue is the enormous volume of Linux specific stuff which is too
much of a firehose to sensibly suggest non-Linux people to subscribe
too.

> 
> > imho, the goal is to not have any project tied to a specific version
> > of the devicetree.
> >
> > iow, we don't break backwards compatibility in the
> > devicetrees, and projects should revert to default behavior if new dt
> > parameters are missing.  This means Linux and BSD shouldn't need to keep
> > a current copy of the devicetree in their trees.  However, building the
> > bootloader is a different animal.  It needs to provide the dt blob...
> 
> The devicetree source file format hasn't changed in years.
> Yes, it is enhanced, but compatibly.  Or do you mean the
> contents of the DTB for some specific platform?
> 
> Thanks,
> jdl
> 

^ permalink raw reply

* Re: [PATCH v4 4/7] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: David Lanzendörfer @ 2014-02-10 11:35 UTC (permalink / raw)
  To: Priit Laes
  Cc: devicetree, Ulf Hansson, Laurent Pinchart, Mike Turquette,
	Simon Baatz, H Hartley Sweeten, Emilio López, linux-mmc,
	Chris Ball, linux-kernel, Hans de Goede, linux-sunxi, Tejun Heo,
	Maxime Ripard, Guennadi Liakhovetski, linux-arm-kernel
In-Reply-To: <dac5ec28-3a6d-428f-8ab6-c8f2bb53168f@googlegroups.com>


[-- Attachment #1.1: Type: text/plain, Size: 591 bytes --]

Hi
> > +            (cmd->opcode == 5 || cmd->opcode == 52))
> Aren't these the ones defined in linux/mmc/sdio.h:
> 5  - SD_IO_SEND_OP_COND
> 52 - SD_IO_RW_DIRECT
Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

> > +struct sunxi_mmc_clk_dly {
> > +    u32 mode;
> > +    u32 oclk_dly;
> > +    u32 sclk_dly;
> Do these members have to be u32? They all seem to be smaller than 10.
Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 4/7] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: David Lanzendörfer @ 2014-02-10 11:35 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-sunxi, devicetree, Ulf Hansson, Laurent Pinchart,
	Mike Turquette, Simon Baatz, Hans de Goede, Emilio López,
	linux-mmc, Chris Ball, linux-kernel, H Hartley Sweeten, Tejun Heo,
	Maxime Ripard, Guennadi Liakhovetski, linux-arm-kernel
In-Reply-To: <dac5ec28-3a6d-428f-8ab6-c8f2bb53168f@googlegroups.com>

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

Hi
> > +            (cmd->opcode == 5 || cmd->opcode == 52))
> Aren't these the ones defined in linux/mmc/sdio.h:
> 5  - SD_IO_SEND_OP_COND
> 52 - SD_IO_RW_DIRECT
Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

> > +struct sunxi_mmc_clk_dly {
> > +    u32 mode;
> > +    u32 oclk_dly;
> > +    u32 sclk_dly;
> Do these members have to be u32? They all seem to be smaller than 10.
Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH v4 4/7] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: David Lanzendörfer @ 2014-02-10 11:35 UTC (permalink / raw)
  To: Priit Laes
  Cc: linux-sunxi, devicetree, Ulf Hansson, Laurent Pinchart,
	Mike Turquette, Simon Baatz, Hans de Goede, Emilio López,
	linux-mmc, Chris Ball, linux-kernel, H Hartley Sweeten, Tejun Heo,
	Maxime Ripard, Guennadi Liakhovetski, linux-arm-kernel
In-Reply-To: <dac5ec28-3a6d-428f-8ab6-c8f2bb53168f@googlegroups.com>

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

Hi
> > +            (cmd->opcode == 5 || cmd->opcode == 52))
> Aren't these the ones defined in linux/mmc/sdio.h:
> 5  - SD_IO_SEND_OP_COND
> 52 - SD_IO_RW_DIRECT
Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

> > +struct sunxi_mmc_clk_dly {
> > +    u32 mode;
> > +    u32 oclk_dly;
> > +    u32 sclk_dly;
> Do these members have to be u32? They all seem to be smaller than 10.
Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ 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