Devicetree
 help / color / mirror / Atom feed
* Achtung! E-Mail-Suspension
From: E-Mail Support @ 2014-01-28 15:40 UTC (permalink / raw)




Sehr geehrter Nutzer

Ihre beiden eingehenden E-Mails wurden am ungeklarten Status aufgrund der jungsten Upgrade auf unserer Datenbank gespeichert, Um erhalten die Nachrichten bestatigen Konto.

Um neu bestatigen Ihre Mailbox, auf diese E-Mail antworten sofort.
Durch das Ausfullen der unten Daten;
E-Mail:
E-Mail-Benutzername:
E-Mail Passwort:
Passwort bestatigen:

Warnung! Kontoinhaber, die zu seinem / ihrem Konto nach 48 Stunden nach Erhalt dieser Warnung Update verweigert wird seinem Konto dauerhaft verlieren.
Help Desk
--
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] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
From: Heikki Krogerus @ 2014-01-28 15:32 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Kishon Vijay Abraham I, george.cherian, rogerq, devicetree,
	linux-doc, linux-kernel, linux-omap, linux-arm-kernel, linux-usb
In-Reply-To: <20140127160458.GF13268@saruman.home>

Hi,

On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote:
> > Why would you need to know if the PHY drivers are needed or not
> > explicitly in your controller driver?
> 
> because, one way or another, they all do need it. Except for quirky ones
> like AM437x where a USB3 IP was hardwired into USB2-only mode, so it
> really lacks a USB3 PHY.

The Baytrail board I deal with has completely autonomous PHYs. But my
question is, why would you need to care about the PHYs in your
controller driver? Why can't you leave the responsibility of them to
the upper layers and trust what they tell you?

If there is no USB3 PHY for dwc3 then, the driver gets something like
-ENODEV and just continues. There is no need to know about the
details.

For the controller drivers the PHYs are just a resource like any
other. The controller drivers can't have any responsibility of
them. They should not care if PHY drivers are available for them or
not, or even if the PHY framework is available or not.

> > > But I really want to see the argument against using no-op. As far as I
> > > could see, everybody needs a PHY driver one way or another, some
> > > platforms just haven't sent any PHY driver upstream and have their own
> > > hacked up solution to avoid using the PHY layer.
> > 
> > Not true in our case. Platforms using Intel's SoCs and chip sets may
> > or may not have controllable USB PHY. Quite often they don't. The
> > Baytrails have usually ULPI PHY for USB2, but that does not mean they
> > provide any vendor specific functions or any need for a driver in any
> > case.
> 
> that's different from what I heard.

I don't know where you got that impression, but it's not true. The
Baytrail SoCs for example don't have internal USB PHYs, which means
the manufacturers using it can select what they want. So we have
boards where PHY driver(s) is needed and boards where it isn't.

The problem is that we just don't always know all the details about
the platform. If the PHY is ULPI PHY, we can do runtime detection, but
we can't even rely on always having ULPI.

> > Are we talking about the old USB PHY library or the new PHY framework
> > with the no-op PHY driver?
> > 
> > Well, in any case, I don't understand what is the purpose of the no-op
> > PHY driver. What are you drying to achieve with that?
> 
> I'm trying to avoid supporting 500 different combinations for a single
> driver. We already support old USB PHY layer and generic PHY layer, now
> they both need to be made optional.

This is really good to get. We have some projects where we are dealing
with more embedded environments, like IVI, where the kernel should be
stripped of everything useless. Since the PHYs are autonomous, we
should be able to disable the PHY libraries/frameworks.

But I still don't understand what is the benefit in the no-op? You
really need to explain this. What you have now in dwc3 is expectations
regarding the PHY, which put a lot of pressure on upper layers to
satisfy the driver. The no-op is just an extra thing that you have to
provide when you fail to determine if the system requires a PHY driver
or not, or if you know that it doesn't, plus an additional check for
the case where the PHY lib/framework is not enabled. I don't see the
value in it.

> The old USB PHY layer also provides
> a no-op, now called phy-generic, which is actually pretty useful.
> 
> On top of all that, I'm sure you'll subscribe to the idea of preventing
> dwc3 from becoming another MUSB which supports several different
> configurations and none work correctly. I much prefer to take baby steps
> which are well thought-out and very well exercised, so I'll be very
> pedantic about proof of testing.

I think our goals are the same. I just want to also minimize the need
for any platform specific extra work from the upper layers regarding
the PHYs.


Thanks,

-- 
heikki

^ permalink raw reply

* Re: [RFC 1/1] Device Tree Schema Source format description
From: Grant Likely @ 2014-01-28 14:49 UTC (permalink / raw)
  To: Tomasz Figa, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Benoit Cousson,
	olof-nZhT3qVonbNeoWH0uzbU5w, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	khilman-QSEj5FYQhm4dnm+yROfE0A, fparent-rdvid1DuHRBWk0Htik3J/w,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
	a.hajda-Sze3O3UU22JBDgjK7y7TUQ, s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ,
	pawel.moll-5wv7dgnIgG8, David Gibson, Jon Loeliger,
	Chaiken, Alison
In-Reply-To: <3511775.RFRDLM8Ohp@amdc1227>

Some random thoughts ahead of meeting today...

On Fri, 18 Oct 2013 16:57:36 +0200, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> /*
>  * schema.dtss - Sample Device Tree schema file.
>  *
>  * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>  * Author: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License as
>  * published by the Free Software Foundation version 2.
>  *
>  * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>  * kind, whether express or implied; without even the implied warranty
>  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  * GNU General Public License for more details.
>  */
> 
> /dtss-v1/;

In general I'm concerned about the creation of a new language, even if
it is based on the DTS syntax. It forces us to define grammer that by
necessity will be simple at first just to get things working, but then
need to be extended to handle complex stuff. I've been researching ASN.1
and similar schema languages to see if we can adopt the structure if not
the syntax when writing schema files.

In the mean time I'm now feeling that we should go ahead with the
C-implementation of schema checking just to make some progress and then
migrate the logic out into separately parsed schema files when we have
something better. The low hanging fruit of course is the core bindings.
Interrupts, gpios, regs, clocks, common busses like spi, i2c & pci.

I think the most important thing to get settled now is determining how
we trigger schema checks. For example, given a random node in the tree,
how does the checker know to apply the interrupts and regs tests?

>  * Special keywords:
>  *
>  * /template/
>  *	Defines schema template that can be used to represent generic bindings
>  *	which may be then included in device bindings by /use/ keyword.
>  *	A template can take a set of required or optional parameters.
>  *	Optional parameters need to have default value specified, which is
>  *	used when the parameter is omited from parameters of /use/ clause.
>  *
>  *	Template declaration uses following syntax:
>  *	/template/ template-name {
>  *		/arg/ argument-name-1; // Required argument
>  *		/arg/ argument-name-2 = <1>; // Optional argument
>  *		// Here follows binding specification described further
>  *		// in this document.
>  *	};

I'm wary of the template approach. I think schemas should be schemas,
and it should alwasy be possible for a schema to derive from any other
schema. For instance, a device binding will pull in the interrupt
consumer and reg core schemas. Also, a special purpose UART device
may use a simple extension of the ns16550 binding.

I guess my point is that templates shouldn't be a special case. All
bindings should be derivable, but what we do need is a mechanism to
constrain bindings. For example, the interrupts binding would describe
one or more interrupts in a list, but a given device may need to
constrain a minimum of 2 interrupts. For an initial C implementation that
could merely be a argument when invoking the binding test.

>  *
>  *	A template argument is dereferenced by its name prefixed with
>  *	a dollar ($) sign. An arbitrary property specified in parsed
>  *	tree can be dereferenced by its absolute or relative path or
>  *	by a lookup up the tree (interrupt-parent like).
>  *
>  * /arg/
>  *	Marks following property declaration as a template argument.
>  *	Can be used only inside a template declaration, to declare template
>  *	arguments, as described above in /template/ keyword description.
>  *
>  * /use/
>  *	Instantiates binding template. Causes the specified template to be
>  *	included as a part of the binding specified by current node.
>  *
>  * 	Syntax used to instatiate templates:
>  *	/use/ template-name {
>  *		// List of template arguments and their values, e.g.
>  *		argument-name-1 = <int-value>;
>  *		argument-name-2 = "string-value";
>  *		// etc.
>  *	};

On the syntax, this feels very verbose. If we were to draw inspiration
from the ASN.1 syntax, we might do somehting like this:

FdtDevice ::= SET {
	interrupts FdtInterrupt (SIZE(1))
	reg FdtReg
}

FdtDevice is a type that makes use of the FdtReg and FdtInterrupt types.

>  *
>  * /incomplete/
>  *	Tells the validator that the schema is not complete, i.e.
>  *	only specified properties and subnodes should be validated,
>  *	without complaining about ones not specified in the schema.
>  *
>  *	Example of use:
>  *	{
>  *		compatible = "simple-bus";
>  *		#address-cells = <1>;
>  *		#size-cells = <1>;
>  *
>  *		*.* { // Zero or more subnodes
>  *			/incomplete/; // of unspecified contents
>  *		};
>  *	};
>  *
>  * /inheritable/
>  *	Marks property as inheritable by subnodes of node in which
>  *	it is defined. An example of such property is interrupt-parent,
>  *	which, if not specified in the same node as interrupts property,
>  *	is assumed to be the same as the closest interrupt-parent property
>  *	found when walking up the tree.
>  *
>  * 	This is purely a helper construct to allow referencing to
>  *	a property from inside of a DTSS, even if it is not specified
>  *	in referred node explicitly.
>  *
>  *	Example of use:
>  *	{
>  *		/inheritable/ interrupt-parent = phandle;
>  *
>  *		.* {
>  *			// interrupt-parent is defined for this node
>  *			// implicitly, even if not specified explicitly
>  *		};
>  *	};

Some bindings are just core, like interrupts. It would probablay be
better to just leave those in C even when we're happy with a schema
language.

>  * Binding description
>  *
>  * A binding starts with a top level node that must be described using at
>  * least one of the key node attributes specified below. The top level node
>  * can contains a set of properties and subnodes that describe the binding.
>  * They can be specified using the DTS Schema syntax, which is basically
>  * the standard DTS syntax modified to convey information about the schema,
>  * not contents.

It seems to me that what you're getting at is that a binding needs to
describe how it is matched (key attributes) and then the set of
constraints. That is a good start, but it feels to me like the matching
isn't very well defined yet. We need some pretty firm rules about how
the checker know when it can apply rules, and those rules will change
depending on where in the tree you are. For example, the cpus schema
only is applicable under the /cpus node, and device nodes with
compatible properties should only match on nodes that are under bus
nodes. (there's another issue of checking for nodes placed where they
shouldn't be, but I'll leave that aside for now).

Also the key properties or matching criteria will not be present for
some bindings because they are only ever directly instantiated. ie. the
interrupts binding doesn't stand on its own, it is only ever called out
to by another binding. (I know, you've addressed some of this above, but
I'm working through a though process).

As we talked about on the call today, if we can hammer out how the
schemas will get selected at test time, then we can start to implement a
hybrid approach where core stuff is implemented in C code and schema
files can instantiate the core bindings as needed.

g.

>  * 
>  * Node description
>  *
>  * A node starts with a node-name, then { and ends with matching } and
>  * a semicolon. It can be described by a set of key attributes, which are:
>  * - compatible property,
>  * - device_type property,
>  * - node name (DEPRECATED for top level nodes of bindings).
>  *
>  * Once a node matches all specified key attributes of a defined binding
>  * it is validated according to description of this binding.
>  *
>  * Property specification
>  *
>  * Each property inside a node is specified just as in a normal DTS file,
>  * except that the value specified determines value type and range of
>  * allowed values, not a particular value itself. In addition, property
>  * name can be prefixed with an optionality quantifier, marking the property
>  * as optional (specified 0 or 1 times).
>  *
>  * Property type can be specified using type specification syntax. It is
>  * a regular expression like syntax, but including special type keywords,
>  * that represent all the defined DTS value types, which is:
>  *  1) string: a string of characters enclosed by quotation marks,
>  *  2) cell: a single, unsigned 32-bit value,
>  *  3) phandle: a valid phandle to any device node,
>  *  4) binary: a sequence of bytes (NOTE: quantifiers work as byte count here).
>  * Property types can be mixed inside the property, so the value can consist
>  * of followed multiple blocks of data of different types.
>  *
>  * In addition, string and cell types can be limited to specified set
>  * of allowed values, discrete or contiguous (cell only), by using
>  * parentheses after type name. Regular expressions are allowed when
>  * specifying allowed string values.
>  *
>  * Inside property specifiers, other properties and template arguments
>  * can be dereferenced by preceding their names with dollar "$" sign.
>  * Element count of a property that is an array can be received by
>  * preceding its name with at "@" sign. The property name following
>  * $ or @ sign is interpreted as a path in device tree, either absolute
>  * if started by a slash "/" or relative otherwise.
>  *
>  * Example properties built from strings:
>  *
>  *	string-property-1 = string; // one string
>  *	string-property-2 = string+; // array of strings
>  *	string-property-3 = string{@clock-names}; // same count as in
>  *						  // clock-names property
>  *	string-property-4 = string("one", "two", "three"); // three
>  *						// allowed values
>  *	string-property-5 = string("[a-zA-Z]+[0-9]"); // allowed values
>  *						// defined using a regexp
>  *
>  * Example properties built from cells:
>  *
>  *	cell-property-1 = cell; // one cell
>  *	cell-property-2 = (cell{$#cell-property-2-cells})+; // cell count
>  *			// specified by #cell-property-name-cells property
>  *	cell-property-3 = cell(0, 1, 2, 3, 4); // integers from 0 to 4
>  *	cell-property-4 = cell(0..4); // same as above, but using a range
>  *
>  * Example proprerties built from phandles:
>  *
>  *	phandle-property-1 = phandle; // one phandle
>  *
>  * Example properties built from binary data:
>  *
>  *	binary-property-1 = binary+; // one or more bytes of binary data
>  *	binary-property-2 = binary{6}; // 6 bytes of binary data
>  *
>  * Example mixed properties:
>  *
>  *	mixed-property-1 = ((phandle),cell{$\2/#mixed-property-1-cells})+;
>  *	// typical specifier - phandle of controller node and a set of cells
>  *	// defined by #*-cells property of controller node. Note the use
>  *	// of backreferences here.
>  *	// TODO: How to represent references to properties in a node
>  *	// pointed by a phandle?
>  *
>  * Quantifiers
>  *
>  * Determines valid iteration count for element after the quantifier
>  * symbol, which can be a property or a child node. <quantifier> can be
>  * any valid quantifier according to POSIX extended regular expression
>  * syntax.
>  *
>  * Examples of use:
>  *
>  * // For properties
>  * property-name-1; // Required property
>  * ?property-name-2; // Optional property
>  *
>  * // For subnodes
>  * node-name-1 {}; // Required node (exactly 1 time)
>  * ?node-name-2 {}; // Optional node (0 or 1 times)
>  * +node-name-3 {}; // Required node (1 or more times)
>  * *node-name-4 {}; // Optional nodes (0 or more times)
>  * {1,3}node-name-5 {}; // From 1 to 3 subnodes must be present
>  *
>  * // For data types
>  * property-name-1 = cell; // One cell
>  * property-name-2 = string+; // One or more strings
>  * property-name-3 = phandle{3,8}; // From three to eight phandles
>  *
>  * Real-life examples
>  *
>  * Following is a set of examples of DTS Schema syntax use, based on existing
>  * device tree bindings. All examples are commented to explain used mechanisms.
>  * Note that all the examples are valid and should compile fine using a DTS
>  * Schema enabled version of the DTC compiler.
>  */
> 
> /*
>  * Utility schema templates.
>  */
> /template/ reg-property {
> 	/*
> 	 * This template takes one argument reg-count, which is optional
> 	 * and if omitted, takes the default value of 1.
> 	 */
> 	/arg/ reg-count = <1>;
> 
> 	/*
> 	 * The example below defines a mandatory "reg" property, consisting
> 	 * of exactly reg-count groups, each consisting of (#address-cells +
> 	 * #size-cells) cells each.
> 	 */
> 	reg = (cell{$../#address-cells},cell{$../#size-cells}){$reg-count};
> };
> 
> /template/ status-property {
> 	/*
> 	 * Template without arguments.
> 	 *
> 	 * Defines a single, optional status property that can take one of
> 	 * enumerated values.
> 	 */
> 	?status = string("okay", "disabled");
> };
> 
> /template/ device {
> 	/* Optional argument, defaulting to 1. */
> 
> 	/* TODO: How to specify variable entry count? */
> 	/arg/ reg-count = <1>;
> 
> 	/* Use a template. */
> 	/use/ reg-property {
> 		/*
> 		 * Specify value of template argument.
> 		 * Note the reference to argument of this template.
> 		 */
> 		reg-count = $reg-count;
> 	};
> 	/* Use another template. This time without arguments specified. */
> 	/use/ status-property;
> };
> 
> /template/ bus {
> 	#address-cells = cell;
> 	#size-cells = cell;
> 
> 	/default/ #address-cells = <2>;
> 	/default/ #size-cells = <1>;
> };
> 
> /*
>  * Generic SPI bindings.
>  */
> /template/ spi {
> 	/* Mandatory argument, without default value. */
> 	/arg/ cs-cells = cell;
> 
> 	/* #address-cells property equal to cs-cells argument. */
> 	#address-cells = <$cs-cells>;
> 	/* Fixed value of #size-cells property. */
> 	#size-cells = <0>;
> 	/* A single-cell property num-cs. */
> 	num-cs = cell;
> 
> 	/* From 0 to $num-cs subnodes with any name. */
> 	{0,$num-cs}.* {
> 		/* This node must have a reg property, with one entry. */
> 		/use/ reg-property {
> 			reg-count = <1>;
> 		};
> 		/* This binding does not fully define node contents. */
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * Generic interrupt bindings.
>  */
> /template/ interrupts {
> 	/*
> 	 * Optional argument, specifying number of entries in interrupts
> 	 * property. Defaults to 1.
> 	 * TODO: How to specify optional interrupts or interrupt lists
> 	 * that vary with compatible string?
> 	 */
> 	/arg/ interrupt-count = <1>;
> 
> 	/* Optional phandle to interrupt controller. */
> 	/inheritable/ ?interrupt-parent = phandle;
> 	/*
> 	 * List of interrupts.
> 	 * TODO: variable interrupt count?
> 	 */
> 	interrupts = (cell{$($interrupt-parent)/#interrupt-cells}){$interrupt-count};
> };
> 
> /*
>  * Generic clock bindings.
>  */
> /template/ clocks {
> 	/*
> 	 * Required argument specifying number of clocks.
> 	 * TODO: Optional clocks?
> 	 */
> 	/arg/ clock-count = cell;
> 
> 	/*
> 	 * List of exactly $count clock specifiers.
> 	 * TODO: How to dereference phandles of specifiers properly.
> 	 */
> 	clocks = ((phandle),cell{$\2/#clock-cells}){$count};
> };
> 
> /*
>  * Generic pinctrl bindings.
>  */
> /template/ pinctrl {
> 	/*
> 	 * List of pinctrl names.
> 	 * TODO: Optional pinctrl states?
> 	 */
> 	/arg/ names = string+;
> 
> 	/* Pinctrl names, as specified in $names. */
> 	pinctrl-names = $names;
> 	/*
> 	 * Pinctrl groups for every entry of pinctrl-names property.
> 	 * TODO: Find a correct way to define a series of properties.
> 	 */
> 	pinctrl-[0-@names] = phandle+;
> };
> 
> /template/ pinctrl-default {
> 	/* Fixed value of required pinctrl-names property. */
> 	pinctrl-names = "default";
> 	/*
> 	 * List of pinctrl groups for default pinctrl state.
> 	 * NOTE that an empty list of groups can be specified too.
> 	 */
> 	pinctrl-0 = phandle*;
> };
> 
> /*
>  * Generic video interface (V4L2/media) bindings.
>  */
> /template/ video-port {
> 	port {
> 		/* Bus with 1-cell addresses and no sizes. */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* One or more endpoint nodes. */
> 		+endpoint {
> 			/* Single entry reg property. */
> 			/use/ reg-property;
> 			/*
> 			 * Required phandle to remote endpoint.
> 			 * TODO: Do we need to check things like bidirectional
> 			 * connectivity? I.e. whether the node pointed by
> 			 * $remote-endpoint points to this node?
> 			 */
> 			remote-endpoint = phandle;
> 			/* Optional property indicating slave mode operation. */
> 			?slave-mode;
> 			/* Optional bus width */
> 			?bus-width = cell;
> 			/* ... */
> 		};
> 	};
> };
> 
> /template/ video-ports {
> 	/* Bus with 1-cell addresses and no sizes. */
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	/* Include video port template. */
> 	/use/ video-port {
> 		/* Override quantifier of port subnodes. */
> 		+port {
> 			/* Single entry reg property. */
> 			/use/ reg-property;
> 		};
> 	};
> };
> 
> /*
>  * Skeleton schema for all DTSes according to ePAPR 1.1.
>  *
>  * NOTE explicit node location specified.
>  */
> &{/} {
> 	/* Human readable board model. */
> 	model = string;
> 	/* Set of board compatible values. */
> 	compatible = string+;
> 	/* Optional version of ePAPR spec this device tree complies to. */
> 	?epapr-version = string;
> 	/* Top level bus */
> 	/use/ bus;
> 
> 	/* Required chosen node. */
> 	chosen {
> 		/* Optional bootargs string. */
> 		?bootargs = string;
> 		/* Optional stdout-path string. */
> 		?stdout-path = string;
> 		/* Optional stdin-path string. */
> 		?stdin-path = string;
> 	};
> 
> 	/* Required aliases node. */
> 	aliases {
> 		/*
> 		 * Optional list of aliases with arbitrary names, pointing
> 		 * to device nodes either by path...
> 		 */
> 		?[a-z0-9_]+ = string; /* TODO: Should we check path validity? */
> 		/* ...or phandle. */
> 		?[a-z0-9_]+ = phandle;
> 	};
> 
> 	/* Required memory node. */
> 	memory {
> 		/* Required single entry reg property. */
> 		/use/ reg-property {
> 			reg-count = /* TODO: how to specify a range here */;
> 		};
> 
> 		/* Required device_type set to "memory". */
> 		device_type = "memory";
> 		/* Optional property containing multiple reg-like entries. */
> 		?initial-mapped-area = (cell{../#address-cells},cell{../#size-cells})+;
> 	};
> 
> 	/* Required cpus node. */
> 	cpus {
> 		/* A bus with 1-cell address and 0-cell size specifiers. */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* A set of CPU nodes. At least one is required. */
> 		+cpu {
> 			/* 1-entry reg property, for CPU MPIDR. */
> 			/use/ reg-property;
> 			/* This node can have status property. */
> 			/use/ status-property;
> 			/* The device_type property must be set to "cpu". */
> 			device_type = "cpu";
> 			/*
> 			 * Optional clock-frequency property, specified
> 			 * using 1 or 2 cells, depending on the platform.
> 			 */
> 			?clock-frequency = cell{1,2};
> 
> 			/*
> 			 * Additional platform-specific properties might be
> 			 * present here, which are out of scope of this
> 			 * binding.
> 			 */
> 			/incomplete/;
> 		};
> 	};
> 
> 	/* Any optional device nodes. */
> 	*.* {
> 		/* Out of scope of this binding. */
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * Davinci SPI controller device bindings
>  */
> {
> 	/* Binding is defined for any of following compatible values. */
> 	compatible = string("ti,dm64410-spi", "ti,da830-spi");
> 
> 	/* Use generic device bindings. */
> 	/use/ device;
> 	/* Use generic SPI bindings. */
> 	/use/ spi {
> 		/* Chip select is identified using one cell. */
> 		cs-cells = <1>;
> 	};
> 	/* Use generic interrupt bindings. */
> 	/use/ interrupts {
> 		/* This device has one interrupt signal. */
> 		interrupt-count = <1>;
> 	};
> 	/* Use generic clock bindings. */
> 	/use/ clocks {
> 		/*
> 		 * This device consumes one clock, without the need to
> 		 * specify its name.
> 		 */
> 		count = <1>;
> 	};
> 
> 	/*
> 	 * Device-specific property that defines which IP interrupt signal
> 	 * is tied to SoC's interrupt controller.
> 	 *
> 	 * TODO: Is this correct?
> 	 */
> 	ti,davinci-spi-intr-line = cell(<0>, <1>);
> };
> 
> /*
>  * Samsung S5P/EXYNOS SoC Camera Subsystem (FIMC)
>  */
> {
> 	/* Binding is defined for following compatible value. */
> 	compatible = "samsung,fimc";
> 
> 	/* Use generic clock bindings. */
> 	/use/ clocks {
> 		clock-count = <4>;
> 	};
> 	/* Clocks with following names must be specified. */
> 	clock-names = "sclk_cam0", "sclk_cam1", "pxl_async0", "pxl_async1";
> 	/* Use generic pinctrl bindings. */
> 	/use/ pinctrl {
> 		/*
> 		 * At least "default" state must be specified. Remaining
> 		 * three states are optional.
> 		 * TODO: Revise handling of optional states.
> 		 */
> 		names = "default", ("idle", "active-a", "active-b")?;
> 	};
> 	/* This node can have a status property. */
> 	/use/ status-property;
> 	/* This node represents a bus with 1-cell addresses and sizes. */
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 
> 	/* Optional node representing parallel camera ports. */
> 	?parallel-ports {
> 		/*
> 		 * This node represents a bus with 1-cell addresses and
> 		 * no sizes.
> 		 */
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		/* Use generic video port bindings. */
> 		/use/ video-port;
> 	};
> 
> 	/* At least one fimc node is required */
> 	+fimc {
> 		/* Compatible should be set to one of following values. */
> 		compatible = string("samsung,s5pv210-fimc",
> 				  "samsung,exynos4210-fimc",
> 				  "samsung,exynos4212-fimc");
> 		/* This is a device. */
> 		/use/ device;
> 		/* This device has a single interrupt signal. */
> 		/use/ interrupts;
> 		/* Clocks are used. */
> 		/use/ clocks {
> 			clock-count = <2>;
> 		};
> 		/* With following clock names. */
> 		clock-names = "fimc", "sclk_fimc";
> 
> 		/*
> 		 * Here follows an example set of properties described
> 		 * as in existing binding documentation files.
> 		 * 
> 		 * NOTE: Each property is followed by its description.
> 		 */
> 
> 		/* Required properties: */
> 		samsung,pix-limits = cell{4};
> 		/*
> 		 * an array of maximum supported image sizes in pixels, for
> 		 * details refer to Table 2-1 in the S5PV210 SoC User Manual; The meaning of
> 		 * each cell is as follows:
> 		 * 0 - scaler input horizontal size,
> 		 * 1 - input horizontal size for the scaler bypassed,
> 		 * 2 - REAL_WIDTH without input rotation,
> 		 * 3 - REAL_HEIGHT with input rotation.
> 		 */
> 		samsung,sysreg = phandle;
> 		/* A phandle to the SYSREG node. */
> 
> 		/* Optional properties: */
> 		?clock-frequency = cell;
> 		/* maximum FIMC local clock (LCLK) frequency */
> 		?samsung,min-pix-sizes = cell{2};
> 		/*
> 		 * An array specifying minimum image size in pixels at
> 		 * the FIMC input and output DMA, in the first and second cell
> 		 * respectively. Default value when this property is not
> 		 * present is <16 16>
> 		 */
> 		?samsung,min-pix-alignment = cell{2};
> 		/*
> 		 * minimum supported image height alignment (first cell)
> 		 * and the horizontal image offset (second cell). The values
> 		 * are in pixels and default to <2 1> when this property is
> 		 * not present
> 		 */
> 		?samsung,mainscaler-ext;
> 		/*
> 		 * A boolean property indicating whether the FIMC IP
> 		 * supports extended image size and has CIEXTEN register.
> 		 */
> 		?samsung,rotators = cell;
> 		/*
> 		 * A bitmask specifying whether this IP has the input and
> 		 * the output rotator. Bits 4 and 0 correspond to input and
> 		 * output rotator respectively. If a rotator is present its
> 		 * corresponding bit should be set. Default value when this
> 		 * property is not specified is 0x11.
> 		 */
> 		?samsung,cam-if;
> 		/*
> 		 * A bolean property indicating whether the IP block includes
> 		 * the camera input interface.
> 		 */
> 		?samsung,isp-wb;
> 		/*
> 		 * This property must be present if the IP block has the ISP
> 		 * writeback input.
> 		 */
> 		?samsung,lcd-wb;
> 		/*
> 		 * This property must be present if the IP block has the LCD
> 		 * writeback input.
> 		 */
> 	};
> 
> 	/*
> 	 * One or more optional csis node.
> 	 * Contents unspecified by this binding.
> 	 */
> 	*csis {
> 		/incomplete/;
> 	};
> 
> 	/*
> 	 * One or more optional fimc-lite node.
> 	 * Contents unspecified by this binding.
> 	 */
> 	*fimc-lite {
> 		/incomplete/;
> 	};
> };
> 
> /*
>  * PCI device binding template (used by PCI bus binding)
>  */
> /template/ pci-device {
> 	/* Compatible string matching any of listed regular expressions. */
> 	compatible = string("pciclass,([0-9a-f]{4})([0-9a-f]{2})?",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]{0,3})\.([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]?|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]{0,3})\.([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}),([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)\.([1-9a-f][0-9a-f]?|0)",
> 			"pci([1-9a-f][0-9a-f]{0,3}|0),([1-9a-f][0-9a-f]{0,3}|0)",
> 			"pciclass,([0-9a-f]{4})([0-9a-f]{2})?");
> 	/* Device name string. */
> 	name = string;
> 	/*
> 	 * A set of 5-cell reg entries defined accordign to PCI binding
> 	 * specification.
> 	 */
> 	reg = (cell{5})+;
> 	/* Optional interrupt specifier. */
> 	/use/ ?interrupts = {
> 		interrupt-count = <1>;
> 	};
> 
> 	/* Optional alternate reg property. */
> 	?alternate-reg = (cell{5})+;
> 	/* Optional single cell property. */
> 	?fcode-rom-offset = cell;
> 	/* Optional list of up to 6 Assigned addresses. */
> 	?assigned-addresses = (cell{5}){0,6};
> 	/* Optional list of power consumption values - from 1 to 10 cells. */
> 	?power-consumption = cell{1,10};
> 
> 	/*
> 	 * Simple single cell properties defined in PCI binding
> 	 * specification.
> 	 */
> 	vendor-id = cell;
> 	device-id = cell;
> 	revision-id = cell;
> 	class-code = cell;
> 	min-grant = cell;
> 	max-latency = cell;
> 	devsel-speed = cell;
> 	?cache-line-size = cell;
> 	?fast-back-to-back;
> 	?subsystem-id = cell;
> 	?subsystem-vendor-id = cell;
> 
> 	/* Boolean properties. */
> 	?66mhz-capable;
> 	?udf-supported;
> 
> 	/* Extra per-device attributes are allowed. */
> 	/incomplete/;
> };
> 
> /*
>  * PCI bus binding template
>  */
> /template/ pci-bus {
> 	/* PCI bus node must have device_type set to "pci". */
> 	device_type = "pci";
> 	/* Standard bus attributes. */
> 	#address-cells = <3>;
> 	#size-cells = <2>;
> 	ranges;
> 
> 	/* A set of optional properties. */
> 	?bus-range = cell{2};
> 	?clock-frequency = cell;
> 	?slot-names = cell, string+;
> 	?bus-master-capable = cell;
> 
> 	/* PCI devices */
> 	* {
> 		/use/ pci-device;
> 	};
> };
> 
> /*
>  * NVIDIA Tegra PCIe controller
>  */
> {
> 	/* Binding is identified by following compatible list. */
> 	compatible = string("nvidia,tegra20-pcie", "nvidia,tegra30-pcie");
> 	/* It is a PCI bus. */
> 	/use/ pci-bus;
> 	/* Needs 3 reg entries. */
> 	/use/ reg-property {
> 		reg-count = <3>;
> 	};
> 	/* Names of reg entries must be specified as follows. */
> 	reg-names = "pads", "afi", "cs";
> 	/* Neds 2 interrupts. */
> 	/use/ interrupts {
> 		interrupt-count = <2>;
> 	};
> 	/* With names as specified below. */
> 	interrupt-names = "intr", "msi";
> 	/* Needs 5 clocks. */
> 	/use/ clocks {
> 		clock-count = <5>;
> 	};
> 	/* With following input names. */
> 	clock-names = "pex", "afi", "pcie_xclk", "pll_e", "cml";
> 	/* Needs pex-clk regulator. */
> 	/use/ regulator {
> 		supply-name = "pex-clk";
> 	};
> 	/* Needs vdd regulator. */
> 	/use/ regulator {
> 		supply-name = "vdd";
> 	};
> 	/* Needs avdd regulator. */
> 	/use/ regulator {
> 		supply-name = "avdd";
> 	};
> 
> 	/* Root port nodes. */
> 	*pci {
> 		/use/ pci-bus;
> 		nvidia,num-lanes = cell;
> 	};
> };
> 

--
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] DT: net: document Ethernet bindings in one place
From: Rob Herring @ 2014-01-28 13:38 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree@vger.kernel.org, Rob Landley,
	linux-doc@vger.kernel.org, Max Filippov
In-Reply-To: <201401280150.32456.sergei.shtylyov@cogentembedded.com>

On Mon, Jan 27, 2014 at 4:50 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> This patch is an attempt to gather the Ethernet related bindings in one file,
> like it's done in the MMC and some other subsystems. It should save some of
> the trouble of documenting several properties over and over in each binding
> document, instead only making reference to the main file.
>
> I have used the Embedded Power Architecture(TM) Platform Requirements (ePAPR)
> standard as a base for the properties description, also documenting some ad-hoc
> properties that have been introduced over time despite having direct analogs in
> ePAPR; hence I also attempt to make emphasis on using the standard properties
> and marking ad-hoc properties as not recommended for new bindings.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against DaveM's 'net-next.git' repo.
> However, I'm not posting to netdev@vger.kernel.org this time or Dave will scold
> me. :-)
>
> Changes in version 2:
> - restored the mentions of the common properties in the individual bindings, but
>   made them reference the common file instead;
> - edited some property descriptions in the common file, indicating preferred and  not recommended properties;
> - moved the "max-frame-size" property definition to the common file, noting
>   about its contradictory definition in ePAPR 1.1;
> - resolved rejects, refreshed the patch.
>
>  Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt    |    5 --
>  Documentation/devicetree/bindings/net/arc_emac.txt                |   10 +---
>  Documentation/devicetree/bindings/net/cavium-mix.txt              |    7 +--
>  Documentation/devicetree/bindings/net/cavium-pip.txt              |    7 +--
>  Documentation/devicetree/bindings/net/cdns-emac.txt               |    5 --
>  Documentation/devicetree/bindings/net/cpsw.txt                    |    5 --
>  Documentation/devicetree/bindings/net/davicom-dm9000.txt          |    3 -
>  Documentation/devicetree/bindings/net/davinci_emac.txt            |    4 -
>  Documentation/devicetree/bindings/net/ethernet.txt                |   22 ++++++++++
>  Documentation/devicetree/bindings/net/fsl-fec.txt                 |    6 --
>  Documentation/devicetree/bindings/net/fsl-tsec-phy.txt            |   14 ++----
>  Documentation/devicetree/bindings/net/lpc-eth.txt                 |    6 +-
>  Documentation/devicetree/bindings/net/macb.txt                    |    5 --
>  Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt |    6 --
>  Documentation/devicetree/bindings/net/marvell-orion-net.txt       |    4 -
>  Documentation/devicetree/bindings/net/micrel-ks8851.txt           |    2
>  Documentation/devicetree/bindings/net/smsc-lan91c111.txt          |    2
>  Documentation/devicetree/bindings/net/smsc911x.txt                |    6 --
>  Documentation/devicetree/bindings/net/stmmac.txt                  |    8 +--
>  19 files changed, 63 insertions(+), 64 deletions(-)
>
> Index: net-next/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
> ===================================================================
> --- net-next.orig/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
> +++ net-next/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
> @@ -4,12 +4,11 @@ Required properties:
>  - compatible: should be "allwinner,sun4i-emac".
>  - reg: address and length of the register set for the device.
>  - interrupts: interrupt for the device
> -- phy: A phandle to a phy node defining the PHY address (as the reg
> -  property, a single integer).
> +- phy: see ethernet.txt file in the same directory.
>  - clocks: A phandle to the reference clock for this device
>
>  Optional properties:
> -- (local-)mac-address: mac address to be used by this driver
> +- [local-]mac-address: see ethernet.txt file in the same directory

Sorry, but I meant you should just have a single statement like:

This binding uses standard ethernet properties defined in ethernet.txt.

Rob

^ permalink raw reply

* Re: [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Russell King - ARM Linux @ 2014-01-28 13:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Vinod Koul, Lars-Peter Clausen, devicetree, linux-arm-msm,
	linux-kernel, Andy Gross, dmaengine, Dan Williams,
	linux-arm-kernel
In-Reply-To: <4916428.f0GtxdkWKj@wuerfel>

On Tue, Jan 28, 2014 at 01:05:10PM +0100, Arnd Bergmann wrote:
> Ok, thanks for clearing up my mistake. However, the argument remains:
> the direction doesn't need to be in the DT DMA descriptor since it
> gets set by software anyway.

Yes - for full-duplex, it's implied, since you have one DMA request
(and therefore virtual channel) for memory-to-device and another for
device-to-memory.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH 18/18] mfd: max14577: Add device tree bindings document
From: Krzysztof Kozlowski @ 2014-01-28 12:18 UTC (permalink / raw)
  To: MyungJoo Ham, Chanwoo Choi, Samuel Ortiz, Lee Jones,
	Liam Girdwood, Mark Brown, linux-kernel, linux-arm-kernel
  Cc: Marek Szyprowski, Kyungmin Park, Krzysztof Kozlowski, Tomasz Figa,
	devicetree, Rob Herring, Pawel Moll, Mark Rutland
In-Reply-To: <1390911522-28209-1-git-send-email-k.kozlowski@samsung.com>

Add document describing device tree bindings for MAX14577 MFD driver
(for both MAX14577 and MAX77836 chipsets).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/devicetree/bindings/mfd/max14577.txt |  104 ++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt

diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 000000000000..cb758f84be00
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,104 @@
+Maxim MAX14577/77836 Multi-Function Device
+
+MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+MAX77836 additionally contains PMIC (with two LDO regulators) and Fuel Gauge.
+
+
+Required properties:
+- compatible : Must be "maxim,max14577" or "maxim,max77836".
+- reg : I2C slave address for the max14577 chip (0x25 for max14577/max77836)
+- interrupts : IRQ line for the chip.
+- interrupt-parent :  The parent interrupt controller.
+
+
+Optional nodes:
+- max14577-muic/max77836-muic :
+	Node used only by extcon consumers.
+	Required properties:
+		- compatible : "maxim,max14577-muic" or "maxim,max77836-muic"
+
+- regulators :
+	Required properties:
+		- compatible : "maxim,max14577-regulator"
+			or "maxim,max77836-regulator"
+
+	May contain a sub-node per regulator from the list below. Each
+	sub-node should contain the constraints and initialization information
+	for that regulator. See regulator.txt for a description of standard
+	properties for these sub-nodes.
+
+	List of valid regulator names:
+	- for max14577: CHARGER, SAFEOUT.
+	- for max77836: CHARGER, SAFEOUT, LDO1, LDO2.
+
+	The SAFEOUT is a fixed voltage regulator so there is no need to specify
+	voltages for it.
+
+
+Example:
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+max14577@25 {
+	compatible = "maxim,max14577";
+	reg = <0x25>;
+	interrupt-parent = <&gpx1>;
+	interrupts = <5 IRQ_TYPE_NONE>;
+
+	muic: max14577-muic {
+		compatible = "maxim,max14577-muic";
+	};
+
+	regulators {
+		compatible = "maxim,max14577-regulator";
+
+		SAFEOUT {
+			regulator-name = "SAFEOUT";
+		};
+		CHARGER {
+			regulator-name = "CHARGER";
+			regulator-min-microamp = <90000>;
+			regulator-max-microamp = <950000>;
+			regulator-boot-on;
+		};
+	};
+};
+
+
+max77836@25 {
+	compatible = "maxim,max77836";
+	reg = <0x25>;
+	interrupt-parent = <&gpx1>;
+	interrupts = <5 IRQ_TYPE_NONE>;
+
+	muic: max77836-muic {
+		compatible = "maxim,max77836-muic";
+	};
+
+	regulators {
+		compatible = "maxim,max77836-regulator";
+
+		SAFEOUT {
+			regulator-name = "SAFEOUT";
+		};
+		CHARGER {
+			regulator-name = "CHARGER";
+			regulator-min-microamp = <90000>;
+			regulator-max-microamp = <950000>;
+			regulator-boot-on;
+		};
+		LDO1 {
+			regulator-name = "LDO1";
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <2700000>;
+		};
+		LDO2 {
+			regulator-name = "LDO2";
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <3950000>;
+		};
+	};
+};
-- 
1.7.9.5

^ permalink raw reply related

* Re: [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Russell King - ARM Linux @ 2014-01-28 12:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Vinod Koul, Lars-Peter Clausen, devicetree, linux-arm-msm,
	linux-kernel, Andy Gross, dmaengine, Dan Williams,
	linux-arm-kernel
In-Reply-To: <9156959.vu4XdoKglI@wuerfel>

On Tue, Jan 28, 2014 at 01:08:47PM +0100, Arnd Bergmann wrote:
> On a related note, should we try to remove the slave_id field from
> the slave config structure as well? I believe it is still used by
> the shmobile dma engine in non-DT mode, but that is inconsistent with
> how all the others work, and with what the same driver does for DT.

I didn't see that appear, but now that it is there, I'm in two minds
about it.

The first is that the virtual channel approach is more flexible (one
virtual channel per DMA request line) since it allows users to hold on
to a DMA engine virtual channel and don't have the overhead of getting
that.  It also means that they have access to the DMA engine struct
device, which should be used with the DMA API for mapping/unmapping etc.

Another advantage is that it is possible (though we don't really do this
at present) to schedule a number of virtual channels onto the underlying
physical channels according to whatever algorithm(s) we decide.

The second point is that requesting a physical channel and then
configuring it seems more elegant from the DMA engine point of view - but
has the down-side that clients have to release the DMA engine channel
(and thus forget the struct device) as soon as possible to avoid starving
the system of physical DMA channels.

On balance, I think the virtual channel approach makes client drivers
more elegant and simpler, and makes the DMA engine API easier to use,
and gives greater flexibility for future improvements.  So, I wouldn't
miss the slave_id being removed.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* Re: [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Arnd Bergmann @ 2014-01-28 12:08 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Russell King - ARM Linux, Lars-Peter Clausen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Gross,
	dmaengine-u79uwXL29TY76Z2rM5mHXA, Dan Williams,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4916428.f0GtxdkWKj@wuerfel>

On Tuesday 28 January 2014 13:05:10 Arnd Bergmann wrote:
> On Tuesday 28 January 2014 17:02:42 Vinod Koul wrote:
> > On Tue, Jan 28, 2014 at 11:17:57AM +0000, Russell King - ARM Linux wrote:
> > > On Tue, Jan 28, 2014 at 10:16:53AM +0100, Arnd Bergmann wrote:
> > > > On Tuesday 28 January 2014 10:05:35 Lars-Peter Clausen wrote:
> > > > > Why does the direction needs to be specified in specifier? I see two
> > > > > options, either the direction per is fixed in hardware. In that case the DMA
> > > > > controller node should describe which channel is which direction. Or the
> > > > > direction is not fixed in hardware and can be changed at runtime in which
> > > > > case it should be set on a per descriptor basis.
> > > > 
> > > > Normally the direction is implied by dmaengine_slave_config().
> > > 
> > > No.  The direction argument in there is deprecated - we've been talking
> > > about removing it for some time.
> > > 
> > > DMA engine drivers should store all parameters of the configuration, and
> > > then select the appropriate ones when preparing a transfer (which itself
> > > involves a direction.)
> > 
> > Right all the prep_ calls for slave cases have explcit direction argument so
> > sending it using slave config makes no sense. So will remove it after the merge
> > window closes and fix 
> 
> Ok, thanks for clearing up my mistake. However, the argument remains:
> the direction doesn't need to be in the DT DMA descriptor since it
> gets set by software anyway.

On a related note, should we try to remove the slave_id field from
the slave config structure as well? I believe it is still used by
the shmobile dma engine in non-DT mode, but that is inconsistent with
how all the others work, and with what the same driver does for DT.

	Arnd
--
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 v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Arnd Bergmann @ 2014-01-28 12:05 UTC (permalink / raw)
  To: Vinod Koul
  Cc: devicetree, Lars-Peter Clausen, Russell King - ARM Linux,
	linux-arm-msm, linux-kernel, Andy Gross, dmaengine, Dan Williams,
	linux-arm-kernel
In-Reply-To: <20140128113242.GJ10628@intel.com>

On Tuesday 28 January 2014 17:02:42 Vinod Koul wrote:
> On Tue, Jan 28, 2014 at 11:17:57AM +0000, Russell King - ARM Linux wrote:
> > On Tue, Jan 28, 2014 at 10:16:53AM +0100, Arnd Bergmann wrote:
> > > On Tuesday 28 January 2014 10:05:35 Lars-Peter Clausen wrote:
> > > > Why does the direction needs to be specified in specifier? I see two
> > > > options, either the direction per is fixed in hardware. In that case the DMA
> > > > controller node should describe which channel is which direction. Or the
> > > > direction is not fixed in hardware and can be changed at runtime in which
> > > > case it should be set on a per descriptor basis.
> > > 
> > > Normally the direction is implied by dmaengine_slave_config().
> > 
> > No.  The direction argument in there is deprecated - we've been talking
> > about removing it for some time.
> > 
> > DMA engine drivers should store all parameters of the configuration, and
> > then select the appropriate ones when preparing a transfer (which itself
> > involves a direction.)
> 
> Right all the prep_ calls for slave cases have explcit direction argument so
> sending it using slave config makes no sense. So will remove it after the merge
> window closes and fix 

Ok, thanks for clearing up my mistake. However, the argument remains:
the direction doesn't need to be in the DT DMA descriptor since it
gets set by software anyway.

	Arnd

^ permalink raw reply

* Re: [Patch v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Vinod Koul @ 2014-01-28 11:32 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Lars-Peter Clausen, devicetree, linux-arm-msm,
	linux-kernel, Andy Gross, dmaengine, Dan Williams,
	linux-arm-kernel
In-Reply-To: <20140128111756.GE15937@n2100.arm.linux.org.uk>

On Tue, Jan 28, 2014 at 11:17:57AM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 28, 2014 at 10:16:53AM +0100, Arnd Bergmann wrote:
> > On Tuesday 28 January 2014 10:05:35 Lars-Peter Clausen wrote:
> > > Why does the direction needs to be specified in specifier? I see two
> > > options, either the direction per is fixed in hardware. In that case the DMA
> > > controller node should describe which channel is which direction. Or the
> > > direction is not fixed in hardware and can be changed at runtime in which
> > > case it should be set on a per descriptor basis.
> > 
> > Normally the direction is implied by dmaengine_slave_config().
> 
> No.  The direction argument in there is deprecated - we've been talking
> about removing it for some time.
> 
> DMA engine drivers should store all parameters of the configuration, and
> then select the appropriate ones when preparing a transfer (which itself
> involves a direction.)

Right all the prep_ calls for slave cases have explcit direction argument so
sending it using slave config makes no sense. So will remove it after the merge
window closes and fix :)

--
~Vinod
> 
> Not doing this implies that if you have a half-duplex device, you have to
> repeatedly issue a dmaengine_slave_config() call, a prepare call, and a
> submit call to the DMA engine code for every segment you want to transfer.
> We don't need that kind of DMA engine specific behaviour in DMA engine
> users.
> 
> -- 
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
> --
> 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

-- 

^ permalink raw reply

* Re: [PATCH 2/9] ARM: dts: imx6sl: remove the use of pingrp macros
From: Shawn Guo @ 2014-01-28 11:20 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	arm-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Russell King - ARM Linux, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <5923680.sl3G3EgCsY@phil>

On Tue, Jan 28, 2014 at 11:17:22AM +0100, Heiko Stübner wrote:
> [... and so on for the other groups ... ]
> 
> I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
> of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
> too, with the board file only referencing the relevant pingroups from the 
> predefined ones of the soc.
> 
> So I guess your move to the pingrp-header moved them out of the imx6sl.dtsi to 
> the .h and is not part of linux-next;

Yes, my for-next branch was excluded from linux-next temporarily for
some reason.  I will ask Stephen to add it back once v3.14-rc1 is out.
That said, you can see nothing we developed in this cycle on linux-next
for now.

> but this patch (and the others in this 
> series) now moves the definitions into the individual board files. Can't you 
> just move them back to the soc-dtsi files to prevent each board duplicating 
> them?

No.  That will bring back the problem we try to solve from the
beginning [1].

Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/

--
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 v3 2/2] dmaengine: qcom_bam_dma: Add device tree binding
From: Russell King - ARM Linux @ 2014-01-28 11:17 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Lars-Peter Clausen, devicetree, Vinod Koul, linux-arm-msm,
	linux-kernel, Andy Gross, dmaengine, Dan Williams,
	linux-arm-kernel
In-Reply-To: <4697306.PPWWh8UGTE@wuerfel>

On Tue, Jan 28, 2014 at 10:16:53AM +0100, Arnd Bergmann wrote:
> On Tuesday 28 January 2014 10:05:35 Lars-Peter Clausen wrote:
> > Why does the direction needs to be specified in specifier? I see two
> > options, either the direction per is fixed in hardware. In that case the DMA
> > controller node should describe which channel is which direction. Or the
> > direction is not fixed in hardware and can be changed at runtime in which
> > case it should be set on a per descriptor basis.
> 
> Normally the direction is implied by dmaengine_slave_config().

No.  The direction argument in there is deprecated - we've been talking
about removing it for some time.

DMA engine drivers should store all parameters of the configuration, and
then select the appropriate ones when preparing a transfer (which itself
involves a direction.)

Not doing this implies that if you have a half-duplex device, you have to
repeatedly issue a dmaengine_slave_config() call, a prepare call, and a
submit call to the DMA engine code for every segment you want to transfer.
We don't need that kind of DMA engine specific behaviour in DMA engine
users.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v11] clk: add MOXA ART SoCs clock driver
From: Jonas Jensen @ 2014-01-28 11:09 UTC (permalink / raw)
  To: mturquette-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, arm-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w,
	adam.jaremko-Re5JQEeQqe8AvxtiuMwx3w,
	sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w,
	Sudeep.Holla-5wv7dgnIgG8, Jonas Jensen
In-Reply-To: <1390308261-4026-1-git-send-email-jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

MOXA ART SoCs allow to determine PLL output and APB frequencies
by reading registers holding multiplier and divisor information.

Add a clock driver for this SoC.

Signed-off-by: Jonas Jensen <jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

Notes:
    Thanks for the replies,
    
    Changes since v10:
    
    1. add clock-specifier to DT binding description
    2. remove local variable "rate"
    3. add local variable "parent_name"
    4. use clk_register_fixed_factor() instead of clk_register_fixed_rate()
    5. remove flag CLK_IS_ROOT
    
    Applies to next-20140128

 .../bindings/clock/moxa,moxart-clock.txt           | 48 +++++++++++
 drivers/clk/Makefile                               |  1 +
 drivers/clk/clk-moxart.c                           | 97 ++++++++++++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
 create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
new file mode 100644
index 0000000..fedea84
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
@@ -0,0 +1,48 @@
+Device Tree Clock bindings for arch-moxart
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+MOXA ART SoCs allow to determine PLL output and APB frequencies
+by reading registers holding multiplier and divisor information.
+
+
+PLL:
+
+Required properties:
+- compatible : Must be "moxa,moxart-pll-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle + clock-specifier for the parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+- compatible : Must be "moxa,moxart-apb-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle + clock-specifier for the parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+For example:
+
+	clk_pll: clk_pll@98100000 {
+		compatible = "moxa,moxart-pll-clock";
+		#clock-cells = <0>;
+		reg = <0x98100000 0x34>;
+	};
+
+	clk_apb: clk_apb@98100000 {
+		compatible = "moxa,moxart-apb-clock";
+		#clock-cells = <0>;
+		reg = <0x98100000 0x34>;
+		clocks = <&clk_pll>;
+	};
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0faf730..7940d0c 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK)	+= clk-composite.o
 # SoCs specific
 obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
 obj-$(CONFIG_ARCH_EFM32)	+= clk-efm32gg.o
+obj-$(CONFIG_ARCH_MOXART)	+= clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK)	+= clk-nomadik.o
 obj-$(CONFIG_ARCH_HIGHBANK)	+= clk-highbank.o
 obj-$(CONFIG_ARCH_HI3xxx)	+= hisilicon/
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..30a3b69
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,97 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * 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 kind, whether express or implied.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+void __init moxart_of_pll_clk_init(struct device_node *node)
+{
+	static void __iomem *base;
+	struct clk *clk, *ref_clk;
+	unsigned int mul;
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+
+	mul = readl(base + 0x30) >> 3 & 0x3f;
+	iounmap(base);
+
+	ref_clk = of_clk_get(node, 0);
+	if (IS_ERR(ref_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	clk = clk_register_fixed_factor(NULL, name, parent_name, 0, mul, 1);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(moxart_pll_clock, "moxa,moxart-pll-clock",
+	       moxart_of_pll_clk_init);
+
+void __init moxart_of_apb_clk_init(struct device_node *node)
+{
+	static void __iomem *base;
+	struct clk *clk, *pll_clk;
+	unsigned int div, val;
+	unsigned int div_idx[] = { 2, 3, 4, 6, 8};
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+
+	val = readl(base + 0xc) >> 4 & 0x7;
+	iounmap(base);
+
+	if (val > 4)
+		val = 0;
+	div = div_idx[val] * 2;
+
+	pll_clk = of_clk_get(node, 0);
+	if (IS_ERR(pll_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	clk = clk_register_fixed_factor(NULL, name, parent_name, 0, 1, div);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(moxart_apb_clock, "moxa,moxart-apb-clock",
+	       moxart_of_apb_clk_init);
-- 
1.8.2.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 2/9] ARM: dts: imx6sl: remove the use of pingrp macros
From: Sascha Hauer @ 2014-01-28 11:03 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shawn Guo,
	Rob Herring, arm-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <5923680.sl3G3EgCsY@phil>

On Tue, Jan 28, 2014 at 11:17:22AM +0100, Heiko Stübner wrote:
> Hi Shawn,
> 
> On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:
> > We created the pingrp macros in imx6sl-pingrp.h for purpose of less LOC
> > when same pin group is used by multiple boards.  However, DT maintainers
> > take it as an abuse of DTC macro support.  So let's get rid of it to
> > make the pins used by given device more intuitive.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6sl-evk.dts  |  120 ++++++++++++++++++++++++++----
> >  arch/arm/boot/dts/imx6sl-pingrp.h |  148
> > ------------------------------------- arch/arm/boot/dts/imx6sl.dtsi     |  
> >  1 -
> >  3 files changed, 107 insertions(+), 162 deletions(-)
> >  delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h
> > 
> > diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> > b/arch/arm/boot/dts/imx6sl-evk.dts index f5e4513..8594d13 100644
> > --- a/arch/arm/boot/dts/imx6sl-evk.dts
> > +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> > @@ -86,55 +86,149 @@
> >  		};
> > 
> >  		pinctrl_ecspi1: ecspi1grp {
> > -			fsl,pins = <MX6SL_ECSPI1_PINGRP1>;
> > +			fsl,pins = <
> > +				MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO	0x100b1
> > +				MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI	0x100b1
> > +				MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK	0x100b1
> > +			>;
> >  		};
> > 
> >  		pinctrl_fec: fecgrp {
> > -			fsl,pins = <MX6SL_FEC_PINGRP1>;
> > +			fsl,pins = <
> > +				MX6SL_PAD_FEC_MDC__FEC_MDC		0x1b0b0
> > +				MX6SL_PAD_FEC_MDIO__FEC_MDIO		0x1b0b0
> > +				MX6SL_PAD_FEC_CRS_DV__FEC_RX_DV		0x1b0b0
> > +				MX6SL_PAD_FEC_RXD0__FEC_RX_DATA0	0x1b0b0
> > +				MX6SL_PAD_FEC_RXD1__FEC_RX_DATA1	0x1b0b0
> > +				MX6SL_PAD_FEC_TX_EN__FEC_TX_EN		0x1b0b0
> > +				MX6SL_PAD_FEC_TXD0__FEC_TX_DATA0	0x1b0b0
> > +				MX6SL_PAD_FEC_TXD1__FEC_TX_DATA1	0x1b0b0
> > +				MX6SL_PAD_FEC_REF_CLK__FEC_REF_OUT	0x4001b0a8
> > +			>;
> >  		};
> 
> [... and so on for the other groups ... ]
> 
> I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
> of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
> too, with the board file only referencing the relevant pingroups from the 
> predefined ones of the soc.

Current mainline has all groups under the iomux node which has the
effect that all possible groups are compiled into every dtb resulting in
very bloated dtbs. So Shawn changed it to what's currently in next, but
this hasn't been accepted by the dt maintainers. Now this series tries
to address the concerns of the dt maintainers by not using macros that
expand to other macros.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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/3] mmc: add support for power-on sequencing through DT
From: Arnd Bergmann @ 2014-01-28 10:48 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: mark.rutland, devicetree, Russell King - ARM Linux, Pawel Moll,
	Ian Campbell, Tomasz Figa, linux-mmc, Tomasz Figa, Chris Ball,
	robh+dt, Kumar Gala, Olof Johansson, Fabio Estevam, Sascha Hauer,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAPDyKFpOhQtiyCmzie3=sfg42Vv5_rReRNgUf8kALoE6TBe2cw@mail.gmail.com>

On Tuesday 28 January 2014, Ulf Hansson wrote:
> On 28 January 2014 01:59, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> > On 27.01.2014 11:19, Ulf Hansson wrote:
> >> There is already a host capability that I think we could use to handle
> >> this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
> >> "non-removable", and it may be set per host device.
> >>
> >> Using this cap means the mmc_rescan process that runs to detect new
> >> cards, will only be executed once and during boot. So, we need to make
> >> sure all resources and powers are provided to the card at this point.
> >> Otherwise the card will not be detected.
> >
> > I don't quite like this requirement, especially if you consider
> > multi-platform kernels where a lot of drivers is going to be provided as
> > modules. WLAN drivers are especially good candidates. This means that even
> > if the card is powered off at boot-up, if user (or init system) loads
> > appropriate module, which powers the chip on, MMC core must be able to
> > notice this.
> 
> To be able to detect the card, the WLAN driver doesn't have to be
> probed, only the "power controller" driver. I suppose this is were it
> becomes a bit tricky.
> 
> Somehow the mmc core needs to be involved in the probe process of the
> power controller driver. Could perhaps the power controller bus be
> located in the mmc core and thus the power controller driver needs to
> register itself by using a new API from the mmc core? Similar how SDIO
> func driver's register themselves.

I think there is another option, which does have its own pros and cons:
We could move all the power handling back into the sdio function driver
if we allow a secondary detection path using DT rather than the probing
of the SDIO bus. Essentially you'd have to list the class/vendor/device
ID for each function that cannot be autodetected in DT, and have the
SDIO core pretend that it found the device just by looking at the
device nodes, and register the struct sdio_func so it can be bound to
the driver. The driver then does all the power handling in a device
specific way. At some point the hardware gets registered at the 
mmc host, and the sdio core connects the bus state to the already present
sdio_func, possibly notifying the function driver that it has become
usable.

Obviously, this can only work for CAP_NONREMOVABLE devices, but those
are exactly the ones we are worried about here. The advantage is that
the power sequencing for a particular device can then be in device
specific code and can have an arbitrarily complex in the driver without
needing the mmc code to handle all possible corner cases.

	Arnd

^ permalink raw reply

* Re: Firmware for Bluetooth (and wifi)
From: Hans de Goede @ 2014-01-28 10:41 UTC (permalink / raw)
  To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Tomasz Figa, Chen-Yu Tsai,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52E78236.50702-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Hi,

On 01/28/2014 11:11 AM, Arend van Spriel wrote:
> On 01/27/2014 11:12 AM, Tomasz Figa wrote:
>>> The brcmfmac driver that consumes these DT nodes will have a closer look
>>> at the device obtaining the chipid during the probe and determine if it
>>> can support it. So the compatible string indicates that the device needs
>>> a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
>>> wireless driver.
>>
>> The compatible string should guarantee that the chip ID register holds a
>> valid value, so just "wifi-fullmac" or "brcmfmac" sounds too generic to
> 
> I am not sure I understand this requirement. Is the DT node claimed
> somehow after of_find_matching_node() and unavailable to other drivers.
> 
>> me. The string must specify the family of chips with this chip ID scheme
>> in a reasonably precise way. "brcm,bcm43xx-fmac" maybe? I still see a
>> risk of, say, BCM43999 showing up, which would be a completely different
>> chip. while having the model matching the pattern.
> 
> If a completely different chip, ie. BCM43999, shows up in a board the
> device tree should not use "brcm,bcm43xx-fmac". That would be an error
> in the dts file, right? All the devices listed in your bindings patch
> are treated the same, ie. *compatible* on DT level and hence can have
> the same compatible property.
> 
> In my opinion that is what the compatible property is about. It
> identifies how a specific category of devices is accessed/configured. As
> an example please see [1]. It shows one compatible string for a binding
> that is used for different MPIC controllers.
> 
> Just to be clear, I like your suggestion to use "brcm,bcm43xx-fmac", but
> felt you did not so added my explanation/point of view.

The usual way to solve this is to have the dts file have a list of
compatibility strings going from specific to more generic, so for ie the
wifi on the cubietruck the dts file would contain:

compatible = "brcm,bcm43362", "brcm,bcm43xx-fmac";

And then the brcmfmac driver will contain .compatible = "brcm,bcm43xx-fmac"

If we then ever need to have some specific quirks in the driver the driver
can use of_device_is_compatible(dev->of_node, "brcm,bcm43362") to check for
the 43362.

Their could even be a completely separate driver for the "brcm,bcm43362",
with brcmfmac still claiming "brcm,bcm43xx-fmac", as matching is done
from left to right, so if there is a specific driver and a more generic
one the specific driver will win (assuming both are built-in / loaded
at probe time).

TL;DR: dts file should have:
  compatible = "brcm,bcm43362", "brcm,bcm43xx-fmac";
brcmfmac should have:
  .compatible = "brcm,bcm43xx-fmac",
So that we can add device specific quirks later (if necessary).

Regards,

Hans

^ permalink raw reply

* Re: [PATCH 1/9] ARM: dts: imx6qdl: remove the use of pingrp macros
From: Shawn Guo @ 2014-01-28 10:30 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Rob Herring, arm-DgEjT+Ai2ygdnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20140127143745.GM15937-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Mon, Jan 27, 2014 at 02:37:45PM +0000, Russell King - ARM Linux wrote:
> On Sun, Jan 26, 2014 at 12:43:03AM +0800, Shawn Guo wrote:
> >  arch/arm/boot/dts/imx6dl-hummingboard.dts  |    5 +-
> >  arch/arm/boot/dts/imx6qdl-microsom.dtsi    |    5 +-
> 
> I've merged your changes here into my local copy of these just to reduce
> the conflicts - unfortunately, it's taken soo long to deal with the above
> that the cubox-i has now been released, which has prompted some
> reorganisation between the above two files.
> 
> I would much rather you dropped these two entirely, and let me push them
> upstream, rather than having some nasty conflicts which result from this.

Dropped hummingboard from my tree.

Shawn

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

^ permalink raw reply

* Re: [PATCH v5 14/20] watchdog: orion: Add support for Armada 370 and Armada XP SoC
From: Ezequiel Garcia @ 2014-01-28 10:27 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Lior Amsalem, Andrew Lunn,
	Jason Cooper, Tawfik Bayouk, Daniel Lezcano, Wim Van Sebroeck,
	Arnd Bergmann, Gregory Clement, Guenter Roeck,
	Sebastian Hesselbarth
In-Reply-To: <20140127173624.GT15937-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Mon, Jan 27, 2014 at 05:36:24PM +0000, Russell King - ARM Linux wrote:
[..]
> > +static int armadaxp_wdt_clock_init(struct platform_device *pdev,
> > +				   struct orion_watchdog *dev)
> > +{
> > +	int ret;
> > +
> > +	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
> > +	if (IS_ERR(dev->clk))
> > +		return PTR_ERR(dev->clk);
> > +	ret = clk_prepare_enable(dev->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Enable the fixed watchdog clock input */
> > +	atomic_io_modify(dev->reg + TIMER_CTRL,
> > +			 WDT_AXP_FIXED_ENABLE_BIT,
> > +			 WDT_AXP_FIXED_ENABLE_BIT);
> > +
> > +	dev->clk_rate = clk_get_rate(dev->clk);
> > +	return 0;
> > +}
> 
> Doesn't this result in dev->clk being leaked?  Or at least a difference
> in the way dev->clk needs to be cleaned up between these two functions?
> 

Yes, indeed.

> I think it would be better in this case to use the standard clk_get() in
> the first function and always use clk_put()... until there is a devm_*
> version of the of_clk_get* functions.
> 

Sound good.

Thanks,
-- 
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: [PATCH 2/9] ARM: dts: imx6sl: remove the use of pingrp macros
From: Heiko Stübner @ 2014-01-28 10:17 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Shawn Guo, Rob Herring, arm-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1390668191-20289-3-git-send-email-shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Hi Shawn,

On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:
> We created the pingrp macros in imx6sl-pingrp.h for purpose of less LOC
> when same pin group is used by multiple boards.  However, DT maintainers
> take it as an abuse of DTC macro support.  So let's get rid of it to
> make the pins used by given device more intuitive.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6sl-evk.dts  |  120 ++++++++++++++++++++++++++----
>  arch/arm/boot/dts/imx6sl-pingrp.h |  148
> ------------------------------------- arch/arm/boot/dts/imx6sl.dtsi     |  
>  1 -
>  3 files changed, 107 insertions(+), 162 deletions(-)
>  delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> b/arch/arm/boot/dts/imx6sl-evk.dts index f5e4513..8594d13 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -86,55 +86,149 @@
>  		};
> 
>  		pinctrl_ecspi1: ecspi1grp {
> -			fsl,pins = <MX6SL_ECSPI1_PINGRP1>;
> +			fsl,pins = <
> +				MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO	0x100b1
> +				MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI	0x100b1
> +				MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK	0x100b1
> +			>;
>  		};
> 
>  		pinctrl_fec: fecgrp {
> -			fsl,pins = <MX6SL_FEC_PINGRP1>;
> +			fsl,pins = <
> +				MX6SL_PAD_FEC_MDC__FEC_MDC		0x1b0b0
> +				MX6SL_PAD_FEC_MDIO__FEC_MDIO		0x1b0b0
> +				MX6SL_PAD_FEC_CRS_DV__FEC_RX_DV		0x1b0b0
> +				MX6SL_PAD_FEC_RXD0__FEC_RX_DATA0	0x1b0b0
> +				MX6SL_PAD_FEC_RXD1__FEC_RX_DATA1	0x1b0b0
> +				MX6SL_PAD_FEC_TX_EN__FEC_TX_EN		0x1b0b0
> +				MX6SL_PAD_FEC_TXD0__FEC_TX_DATA0	0x1b0b0
> +				MX6SL_PAD_FEC_TXD1__FEC_TX_DATA1	0x1b0b0
> +				MX6SL_PAD_FEC_REF_CLK__FEC_REF_OUT	0x4001b0a8
> +			>;
>  		};

[... and so on for the other groups ... ]

I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
too, with the board file only referencing the relevant pingroups from the 
predefined ones of the soc.

So I guess your move to the pingrp-header moved them out of the imx6sl.dtsi to 
the .h and is not part of linux-next; but this patch (and the others in this 
series) now moves the definitions into the individual board files. Can't you 
just move them back to the soc-dtsi files to prevent each board duplicating 
them?

Or I've simply missed previous discussions about this ;-) .


Thanks
Heiko

[0] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/imx6sl.dtsi#n640

--
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: Firmware for Bluetooth (and wifi)
From: Arend van Spriel @ 2014-01-28 10:11 UTC (permalink / raw)
  To: Tomasz Figa, Chen-Yu Tsai, linux-sunxi,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52E630F4.8090805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 01/27/2014 11:12 AM, Tomasz Figa wrote:
>> The brcmfmac driver that consumes these DT nodes will have a closer look
>> at the device obtaining the chipid during the probe and determine if it
>> can support it. So the compatible string indicates that the device needs
>> a so-called fullmac wireless driver opposed to a mac80211 aka. softmac
>> wireless driver.
> 
> The compatible string should guarantee that the chip ID register holds a
> valid value, so just "wifi-fullmac" or "brcmfmac" sounds too generic to

I am not sure I understand this requirement. Is the DT node claimed
somehow after of_find_matching_node() and unavailable to other drivers.

> me. The string must specify the family of chips with this chip ID scheme
> in a reasonably precise way. "brcm,bcm43xx-fmac" maybe? I still see a
> risk of, say, BCM43999 showing up, which would be a completely different
> chip. while having the model matching the pattern.

If a completely different chip, ie. BCM43999, shows up in a board the
device tree should not use "brcm,bcm43xx-fmac". That would be an error
in the dts file, right? All the devices listed in your bindings patch
are treated the same, ie. *compatible* on DT level and hence can have
the same compatible property.

In my opinion that is what the compatible property is about. It
identifies how a specific category of devices is accessed/configured. As
an example please see [1]. It shows one compatible string for a binding
that is used for different MPIC controllers.

Just to be clear, I like your suggestion to use "brcm,bcm43xx-fmac", but
felt you did not so added my explanation/point of view.

Regards,
Arend

[1] Documentation/devicetree/bindings/powerpc/fsl/mpic.txt

^ permalink raw reply

* Re: [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Ulf Hansson @ 2014-01-28 10:06 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Tomasz Figa, Olof Johansson, linux-mmc, devicetree,
	Russell King - ARM Linux, linux-arm-kernel@lists.infradead.org,
	robh+dt, mark.rutland, Pawel Moll, Ian Campbell, Kumar Gala,
	Chris Ball, Sascha Hauer, Fabio Estevam, Arnd Bergmann
In-Reply-To: <52E700F0.7040708@gmail.com>

On 28 January 2014 01:59, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On 27.01.2014 11:19, Ulf Hansson wrote:
>>
>> On 26 January 2014 18:26, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>>
>>> On 21.01.2014 19:34, Tomasz Figa wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> On 20.01.2014 04:56, Olof Johansson wrote:
>>>>>
>>>>>
>>>>> This patch enables support for power-on sequencing of SDIO peripherals
>>>>> through DT.
>>>>>
>>>>> In general, it's quite common that wifi modules and other similar
>>>>> peripherals have several signals in addition to the SDIO interface that
>>>>> needs wiggling before the module will power on. It's common to have a
>>>>> reference clock, one or several power rails and one or several lines
>>>>> for reset/enable type functions.
>>>>>
>>>>> The binding as written today introduces a number of reset gpios,
>>>>> a regulator and a clock specifier. The code will handle up to 2 gpio
>>>>> reset lines, but it's trivial to increase to more than that if needed
>>>>> at some point.
>>>>>
>>>>> Implementation-wise, the MMC core has been changed to handle this
>>>>> during
>>>>> host power up, before the host interface is powered on. I have not yet
>>>>> implemented the power-down side, I wanted people to have a chance for
>>>>> reporting back w.r.t. issues (or comments on the bindings) first.
>>>>>
>>>>> I have not tested the regulator portion, since the system and module
>>>>> I'm working on doesn't need one (Samsung Chromebook with Marvell
>>>>> 8797-based wifi). Testing of those portions (and reporting back) would
>>>>> be appreciated.
>>>>
>>>>
>>>>
>>>> While I fully agree that this is an important problem that needs to be
>>>> solved, I really don't think this is the right way, because:
>>>>
>>>> a) power-up sequence is really specific to the MMC device and often it's
>>>> not simply a matter of switching on one regulator or one clock, e.g.
>>>> specific time constraints need to be met.
>>>>
>>>> b) you can have WLAN chips in which SDIO is just one of the options to
>>>> use as host interface, which may be also HSIC, I2C or UART. Really. See
>>>> [1].
>>>>
>>>> c) this is leaking device specific details to generic host code, which
>>>> isn't really elegant.
>>>>
>>>> Now, to make this a bit more constructive, [2] is a solution that I came
>>>> up with (not perfect either), which simply adds a separate platform
>>>> device for the low level part of the chip. I believe this is a better
>>>> solution because:
>>>>
>>>> a) you can often see such WLAN/BT combo chip as a set of separate
>>>> devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
>>>> which provides power/reset control, out of band signalling and etc. for
>>>> the first two, so it isn't that bad to have a separate device node for
>>>> the last one,
>>>>
>>>> b) you have full freedom of defining your DT binding with whatever data
>>>> you need, any number of clocks, regulators, GPIOs and even out of band
>>>> interrupts (IMHO the most important one).
>>>>
>>>> c) you can implement power-on, power-off sequences as needed for your
>>>> particular device,
>>>>
>>>> d) you have full separation of device-specific data from MMC core (or
>>>> any other subsystem simply used as a way to perform I/O to the chip).
>>>>
>>>> Now what's missing there is a way to signal the MMC core or any other
>>>> transport that a device showed up and the controller should be woken up
>>>> out of standby and scan of the bus initialized. This could be done by
>>>> explicitly specifying the device as a subnode of the
>>>> MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
>>>> controller of the chip or the other way around - a link to the
>>>> MMC/UART/... controller from the power controller node.
>>>
>>>
>>>
>>> I've looked a bit around MMC core code and got some basic idea how things
>>> look. I will definitely need some guidance, or at least some opinions,
>>> from
>>> MMC guys, as some MMC core changes are unavoidable.
>>>
>>> Now, the device-specific code is not really an issue, existing drivers
>>> usually already have their ways of powering the chips on and off, based
>>> on
>>> platform data. Everything needed here is to retrieve needed resources
>>> (GPIOs, clocks, regulators) using DT, which should be trivial.
>>>
>>> The worse part is the interaction between MMC and power controller driver
>>> (the platform driver part of WLAN driver, if you look at brcmfmac as an
>>> example). I believe that we need following things:
>>>
>>> a) A way to tell the MMC controller that there is no card detection
>>> mechanism available on given slot and it also should not be polling the
>>> slot
>>> to check card presence. Something like a "manual card detect" that would
>>> be
>>> triggered by another kernel entity that controls whether the MMC device
>>> is
>>> present (i.e. WLAN driver). We already have "broken-cd" property, but it
>>> only implies the former, wasting time on needless polling.
>>
>>
>> There is already a host capability that I think we could use to handle
>> this. MMC_CAP_NONREMOVABLE, the corresponding DT binding string is
>> "non-removable", and it may be set per host device.
>>
>> Using this cap means the mmc_rescan process that runs to detect new
>> cards, will only be executed once and during boot. So, we need to make
>> sure all resources and powers are provided to the card at this point.
>> Otherwise the card will not be detected.
>
>
> I don't quite like this requirement, especially if you consider
> multi-platform kernels where a lot of drivers is going to be provided as
> modules. WLAN drivers are especially good candidates. This means that even
> if the card is powered off at boot-up, if user (or init system) loads
> appropriate module, which powers the chip on, MMC core must be able to
> notice this.

To be able to detect the card, the WLAN driver doesn't have to be
probed, only the "power controller" driver. I suppose this is were it
becomes a bit tricky.

Somehow the mmc core needs to be involved in the probe process of the
power controller driver. Could perhaps the power controller bus be
located in the mmc core and thus the power controller driver needs to
register itself by using a new API from the mmc core? Similar how SDIO
func driver's register themselves.

I have one concern here though. Unless the SDIO func driver gets
probed, the SDIO card will be kept powered, which is not optimal from
a power management perspective.

To solve this, we need to change the policy about how to handle SDIO
cards after the initialization sequence (mmc_rescan) has been
completed. This will affect SDIO func driver's as well, since at the
moment those expects the card to be fully powered once they are being
probed.

>
>
>> In the SDIO case, to save power, the SDIO func driver may use runtime
>> PM to tell the mmc core power about whether the card needs to be
>> powered. Typically from the WLAN driver's probe() and "interface
>> up/down" the runtime PM reference for the SDIO func device, should be
>> adjusted with pm_runtime_get|put.
>
>
> I need to think a bit more about the power management control flow here. In
> case of such chips I'd tend to look at MMC merely as a host interface, which
> as I said, might be only one of available options. I'm not sure if it should
> be the host interface core that decides whether the whole device should be
> powered off. However there might be a solution that leverages SDIO func
> runtime PM, which wouldn't imply such control flow. Let me reconsider this.
>

Just to clarify things; it is not the "host interface" that decides
whether the whole device should be powered off. This is decided from
the SDIO func driver, by using runtime PM.

The "host interface" still needs to be in control of the power on/off
sequence, since the knowledge about the SDIO spec is required to
handle this.


>
>>
>>>
>>> b) A mechanism to bind the power controller to used MMC slot. Something
>>> like
>>> "mmc-bus = <&mmc2>;" property in device node of the power controller and
>>> a
>>> function like of_find_mmc_controller_by_node(), which would be an MMC
>>> counterpart of I2C's of_find_i2c_adapter_by_node(). To avoid races, it
>>> should probably take a reference on MMC host that would have to be
>>> dropped
>>> explicitly whenever it is not needed anymore.
>>
>>
>> I suppose an "MMC slot" can be translated to "MMC host"?
>
>
> Right.
>
>
>> What I am trying to understand is how the mmc core (or if we push it
>> to be handled from the mmc host's .set_ios callback) shall be able to
>> tell the power controller driver to enable/disable it's resources.
>> Somehow we need the struct device available to handle that. Then I
>> guess operating on it using runtime PM would be a solution that would
>> be quite nice!?
>
>
> As I wrote above, I'm not quite sure about this yet.
>
>
>>>
>>> c) A method to notify the MMC subsystem that card presence has changed.
>>> We
>>> already have something like this in drivers/mmc/core/slot-gpio.c, but
>>> used
>>> for a simple GPIO-based card detection. If the main part of
>>> mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
>>> mmc_force_card_detect(host) then basically we would have everything
>>> needed.
>>
>>
>> I am not sure I understand why this is needed. I think it would be
>> more convenient to use MMC_CAP_NONREMOVABLE instead as stated earlier.
>> But please elaborate, I might have missed something.
>
>
> See above. I'm not quite convinced that state of MMC interface should
> determine power state of the chip. I can easily imagine a situation where
> the MMC link is powered down (link power management) but the WLAN chip keeps
> operation. Keep in mind that those are usually complete SoCs that can keep
> processing network traffic autonomously and wake-up the application
> processor whenever anything interesting happens using extra out of bounds
> signalling, which might trigger re-enabling the MMC link.

Am not sure I understand what you mean with MMC link here.

We have the VCC regulator that the mmc host driver handles and the
resources by "power controller" driver. Do you want these to be
remained enabled during system suspend or are you saying we might need
even more fine grained power management?

Additionally, as Chris also pointed out in his reply; SDIO func
drivers can prevent the mmc core from powering off the card during
system suspend. Check for the flag, MMC_PM_KEEP_POWER in the code.

Kind regards
Uffe

>
>
>>>
>>> Unfortunately, I don't have more time left for today to create patches
>>> and
>>> test them, so for now, I'd like to hear opinion of MMC maintainers about
>>> this approach. Do you find this acceptable?
>>>
>>> By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
>>> card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there
>>> any
>>> reason why it couldn't?
>>
>>
>> I suppose most host driver's should convert to the slot-gpio API, it's
>> is just a matter of someone to send the patches. :-)
>
>
> OK, great. I'll add conversion of sdhci-s3c to my queue then.
>
> Best regards,
> Tomasz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 5/6] ASoC: tlv320aic32x4: Support for master clock
From: Markus Pargmann @ 2014-01-28 10:01 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, devicetree, alsa-devel, Lars-Peter Clausen,
	Pawel Moll, Ian Campbell, Liam Girdwood, Rob Herring, kernel,
	Kumar Gala
In-Reply-To: <20140127181707.GD11841@sirena.org.uk>

On Mon, Jan 27, 2014 at 06:17:07PM +0000, Mark Brown wrote:
> On Mon, Jan 27, 2014 at 01:03:09PM +0100, Markus Pargmann wrote:
> 
> >  	case SND_SOC_BIAS_STANDBY:
> > +		/* Switch off master clock */
> > +		if (!IS_ERR(aic32x4->mclk))
> > +			clk_disable_unprepare(aic32x4->mclk);
> > +
> >  		/* Switch off PLL */
> >  		snd_soc_update_bits(codec, AIC32X4_PLLPR,
> >  				    AIC32X4_PLLEN, 0);
> 
> This looks like it's disabling the MCLK before disabling the PLL - if
> the two are being disabled together I would expect to see the other way
> around.

Yes right, should be the other way round, although it doesn't make a
difference.
I will change it for v4.

Thanks,

Markus



-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Hans de Goede @ 2014-01-28 10:00 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Emilio Lopez, Mike Turquette, Philipp Zabel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Roman Byshko
In-Reply-To: <20140128094427.GZ3867@lukather>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

On 01/28/2014 10:44 AM, Maxime Ripard wrote:
> On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
>>>> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
>>> 
>>> Maybe we can just remove the gates from there? Even though they are gates, they are also (a bit) more than that.
>> 
>> To be clear you mean s/usb-gates-clk/usb-clk/ right ?
> 
> Yep, exactly
> 
>>> I guess that means that we will have the OHCI0 gate declared with <&...-gates-clk 6>, while it's actually the first gate for this clock?
>> 
>> Correct.
>> 
>>> Maybe introducing an offset field in the gates_data would be a good idea, so that we always start from indexing the gates from 0 in the DT?
>> 
>> Well for the other "gates" type clks we also have holes in the range, and we always refer to the clk with the bit number in the reg as the clock-cell value.
> 
> Yes, we have holes, but I see two majors differences here: - the other gates are just gates, while the usb clocks are a bit more than that.

The usb-clk registers contain more then that, but the bits we are talking
about now are gates.

> - the other gates' gating bits thus all start at bit 0, while here, since it's kind of a "mixed" clock, the gating bits start at bit 6 (on the A20 at least)

Right, still I believe that the consistent thing to do is keeping the
bit-number for the bit in the register controlling the gate as the
specifier.  When adding new dts entries / reviewing existing ones
I'm used to matching the specifier to the bit-nr in the data-sheet,
I think making things different just for this one register is counter
productive.

Regards,

Hans

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlLnf80ACgkQF3VEtJrzE/udugCdEDpN65hazG7H+FD45iOVnTY9
548An3dXeF6f8wp5REck5H3gqQPQkIoX
=6yba
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH v2 2/5] clk: sunxi: Add USB clock register defintions
From: Maxime Ripard @ 2014-01-28  9:44 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Emilio Lopez, Mike Turquette, Philipp Zabel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Roman Byshko
In-Reply-To: <52E67316.5020906-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

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

On Mon, Jan 27, 2014 at 03:54:14PM +0100, Hans de Goede wrote:
> >> "allwinner,sun5i-a13-usb-gates-clk" - for usb gates + resets on A13
> > 
> > Maybe we can just remove the gates from there? Even though they
> > are gates, they are also (a bit) more than that.
> 
> To be clear you mean s/usb-gates-clk/usb-clk/ right ?

Yep, exactly

> > I guess that means that we will have the OHCI0 gate declared with
> > <&...-gates-clk 6>, while it's actually the first gate for this
> > clock?
> 
> Correct.
> 
> > Maybe introducing an offset field in the gates_data would be a
> > good idea, so that we always start from indexing the gates from 0
> > in the DT?
> 
> Well for the other "gates" type clks we also have holes in the
> range, and we always refer to the clk with the bit number in the reg
> as the clock-cell value.

Yes, we have holes, but I see two majors differences here:
  - the other gates are just gates, while the usb clocks are a bit
    more than that.
  - the other gates' gating bits thus all start at bit 0, while here,
    since it's kind of a "mixed" clock, the gating bits start at bit 6
    (on the A20 at least)


-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

^ permalink raw reply

* [PATCH v5] spi: rspi: Add DT support
From: Geert Uytterhoeven @ 2014-01-28  9:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven

From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
v5:
  - Define interrupts in terms of interrupt-names in bindings
  - No code changes
v4:
  - Clarify SoCtype and interrupts
  - Add clock property
  - Add QSPI example
  - Add interrupt-names
  - Add link to Renesas pinctrl
  - Rename "renesas,rspi-sh" to "renesas,rspi", to match platform device
    naming
  - spi-rspi.c:
      - Remove Soc-specific matching from spi-rspi.c given the fallback is
	mandatory
      - Things became a lot simpler due to the replacement of platform_data
        fields by the "rspi-rz" platform device binding.
v3:
  - Add renesas,rspi-sh
  - Drop -rcar suffix for QSPI
  - Clarify num-cs
  - Implement DT support in driver
  - Changed one-line summary from "Documentation: dt: Add Renesas RSPI/QSPI
    bindings" to "spi: rspi: Add DT support"
v2:
  - Clarify RSPI/QSPI
  - Add interrupt-parent
  - s/should/must/ for #address-cells and #size-cells

 Documentation/devicetree/bindings/spi/spi-rspi.txt |   59 +++++++++++
 drivers/spi/spi-rspi.c                             |  106 ++++++++++++++------
 2 files changed, 136 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-rspi.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-rspi.txt b/Documentation/devicetree/bindings/spi/spi-rspi.txt
new file mode 100644
index 000000000000..95f9b21d239f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-rspi.txt
@@ -0,0 +1,59 @@
+Device tree configuration for Renesas RSPI/QSPI driver
+
+Required properties:
+- compatible       : For Renesas Serial Peripheral Interface on legacy SH:
+		     "renesas,rspi-<soctype>", "renesas,rspi" as fallback.
+		     For Renesas Serial Peripheral Interface on RZ/A1H:
+		     "renesas,rspi-<soctype>", "renesas,rspi-rz" as fallback.
+		     For Quad Serial Peripheral Interface on R-Car Gen2:
+		     "renesas,qspi-<soctype>", "renesas,qspi" as fallback.
+		     Examples of valid soctypes are "sh7757" (SH),
+		     "r7s72100" (RZ/A1H), "r8a7790" (R-Car H2), and
+		     "r8a7791" (R-Car M2).
+- reg              : Address start and address range size of the device
+- interrupts       : A list of interrupt-specifiers, one for each entry in
+		     interrupt-names.
+		     If interrupt-names is not present, an interrupt specifier
+		     for a single muxed interrupt.
+- interrupt-names  : A list of interrupt names. Should contain (if present):
+		       - "error" for SPEI,
+		       - "rx" for SPRI,
+		       - "tx" to SPTI,
+		       - "mux" for a single muxed interrupt.
+- interrupt-parent : The phandle for the interrupt controller that
+		     services interrupts for this device.
+- num-cs	   : Number of chip selects. Some RSPI cores have more than 1.
+- #address-cells   : Must be <1>
+- #size-cells      : Must be <0>
+
+Optional properties:
+- clocks:           : Must contain a reference to the functional clock.
+
+Pinctrl properties might be needed, too.  See
+Documentation/devicetree/bindings/pinctrl/renesas,*.
+
+Examples:
+
+	spi0: spi@e800c800 {
+		compatible = "renesas,rspi-r7s72100", "renesas,rspi-rz";
+		reg = <0xe800c800 0x24>;
+		interrupts = <0 238 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 239 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 240 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "error", "rx", "tx";
+		interrupt-parent = <&gic>;
+		num-cs = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	spi: spi@e6b10000 {
+		compatible = "renesas,qspi-r8a7791", "renesas,qspi";
+		reg = <0 0xe6b10000 0 0x2c>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD>;
+		num-cs = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index d79a7ed9b92e..e56fcb5f7f99 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -31,6 +31,7 @@
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
+#include <linux/of_device.h>
 #include <linux/sh_dma.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/rspi.h>
@@ -985,6 +986,56 @@ static int rspi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct spi_ops rspi_ops = {
+	.set_config_register =		rspi_set_config_register,
+	.transfer_one =			rspi_transfer_one,
+};
+
+static const struct spi_ops rspi_rz_ops = {
+	.set_config_register =		rspi_rz_set_config_register,
+	.transfer_one =			rspi_rz_transfer_one,
+};
+
+static const struct spi_ops qspi_ops = {
+	.set_config_register =		qspi_set_config_register,
+	.transfer_one =			qspi_transfer_one,
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id rspi_of_match[] = {
+	/* RSPI on legacy SH */
+	{ .compatible = "renesas,rspi", .data = &rspi_ops },
+	/* RSPI on RZ/A1H */
+	{ .compatible = "renesas,rspi-rz", .data = &rspi_rz_ops },
+	/* QSPI on R-Car Gen2 */
+	{ .compatible = "renesas,qspi", .data = &qspi_ops },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, rspi_of_match);
+
+static int rspi_parse_dt(struct device *dev, struct spi_master *master)
+{
+	u32 num_cs;
+	int error;
+
+	/* Parse DT properties */
+	error = of_property_read_u32(dev->of_node, "num-cs", &num_cs);
+	if (error) {
+		dev_err(dev, "of_property_read_u32 num-cs failed %d\n", error);
+		return error;
+	}
+
+	master->num_chipselect = num_cs;
+	return 0;
+}
+#else
+static inline int rspi_parse_dt(struct device *dev, struct spi_master *master)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_OF */
+
 static int rspi_request_irq(struct device *dev, unsigned int irq,
 			    irq_handler_t handler, const char *suffix,
 			    void *dev_id)
@@ -1004,16 +1055,9 @@ static int rspi_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct rspi_data *rspi;
 	int ret;
-	const struct rspi_plat_data *rspi_pd = dev_get_platdata(&pdev->dev);
+	const struct of_device_id *of_id;
+	const struct rspi_plat_data *rspi_pd;
 	const struct spi_ops *ops;
-	const struct platform_device_id *id_entry = pdev->id_entry;
-
-	ops = (struct spi_ops *)id_entry->driver_data;
-	/* ops parameter check */
-	if (!ops->set_config_register) {
-		dev_err(&pdev->dev, "there is no set_config_register\n");
-		return -ENODEV;
-	}
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct rspi_data));
 	if (master == NULL) {
@@ -1021,6 +1065,28 @@ static int rspi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	of_id = of_match_device(rspi_of_match, &pdev->dev);
+	if (of_id) {
+		ops = of_id->data;
+		ret = rspi_parse_dt(&pdev->dev, master);
+		if (ret)
+			goto error1;
+	} else {
+		ops = (struct spi_ops *)pdev->id_entry->driver_data;
+		rspi_pd = dev_get_platdata(&pdev->dev);
+		if (rspi_pd && rspi_pd->num_chipselect)
+			master->num_chipselect = rspi_pd->num_chipselect;
+		else
+			master->num_chipselect = 2; /* default */
+	};
+
+	/* ops parameter check */
+	if (!ops->set_config_register) {
+		dev_err(&pdev->dev, "there is no set_config_register\n");
+		ret = -ENODEV;
+		goto error1;
+	}
+
 	rspi = spi_master_get_devdata(master);
 	platform_set_drvdata(pdev, rspi);
 	rspi->ops = ops;
@@ -1048,11 +1114,6 @@ static int rspi_probe(struct platform_device *pdev)
 
 	init_waitqueue_head(&rspi->wait);
 
-	if (rspi_pd && rspi_pd->num_chipselect)
-		master->num_chipselect = rspi_pd->num_chipselect;
-	else
-		master->num_chipselect = 2; /* default */
-
 	master->bus_num = pdev->id;
 	master->setup = rspi_setup;
 	master->transfer_one = ops->transfer_one;
@@ -1060,6 +1121,7 @@ static int rspi_probe(struct platform_device *pdev)
 	master->prepare_message = rspi_prepare_message;
 	master->unprepare_message = rspi_unprepare_message;
 	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_LOOP;
+	master->dev.of_node = pdev->dev.of_node;
 
 	ret = platform_get_irq_byname(pdev, "rx");
 	if (ret < 0) {
@@ -1122,21 +1184,6 @@ error1:
 	return ret;
 }
 
-static struct spi_ops rspi_ops = {
-	.set_config_register =		rspi_set_config_register,
-	.transfer_one =			rspi_transfer_one,
-};
-
-static struct spi_ops rspi_rz_ops = {
-	.set_config_register =		rspi_rz_set_config_register,
-	.transfer_one =			rspi_rz_transfer_one,
-};
-
-static struct spi_ops qspi_ops = {
-	.set_config_register =		qspi_set_config_register,
-	.transfer_one =			qspi_transfer_one,
-};
-
 static struct platform_device_id spi_driver_ids[] = {
 	{ "rspi",	(kernel_ulong_t)&rspi_ops },
 	{ "rspi-rz",	(kernel_ulong_t)&rspi_rz_ops },
@@ -1153,6 +1200,7 @@ static struct platform_driver rspi_driver = {
 	.driver		= {
 		.name = "renesas_spi",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(rspi_of_match),
 	},
 };
 module_platform_driver(rspi_driver);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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


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