Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable
From: Andy Shevchenko @ 2017-04-28 15:37 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Linus Walleij, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
	Rob Herring, Mark Rutland, Russell King - ARM Linux,
	Linux-Renesas, linux-gpio@vger.kernel.org, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <SG2PR06MB11658E6434EB343246ADC9F58A130@SG2PR06MB1165.apcprd06.prod.outlook.com>

On Fri, Apr 28, 2017 at 6:16 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, April 28, 2017, Andy Shevchenko wrote:
>> Had you read the following, esp. Note there?
>>
>> * @PIN_CONFIG_INPUT_ENABLE: enable the pin's input.  Note that this does
>> not
>> *      affect the pin's ability to drive output.  1 enables input, 0
>> disables
>> *      input.
>>
>> For me manual is clearly tells about this settings:
>> "This register enables or disables the input buffer while the output
>> buffer is enabled."
>
>
> But, then if we use "input-enable" to get bi-directional functionality,

It seems you are missing the point from electrical prospective.
Standard pin buffers (electrically) means input buffer and output
buffer that are driven independently in most cases.

Here is one example:
https://electronics.stackexchange.com/questions/96932/internal-circuitry-of-io-ports-in-mcu/96953#96953

(That's what I asked before as a schematic)

> now we need something to replace what we were using "input-enable" for.

No.

> We were using "input-enable" to signal when the pin function that we set also needs to be forcible set to input by the software (once again, because the HW is not smart enough to do it on its own), but is different than the bi-directional functionality (ie, a different register setting).

You are trying to introduce an abstraction, called BiDi, which is
*not* a separate thing from a set of pin properties.

> So, if we replace "bi-directional" with "input-enable" (since logically internally that is what is going on), what do we use for the special pins that the HW manual says "hey, you need to manually set these pins to input with SW because the pin selection HW can't do it correctly)".

Yes.
BiDi is an alias to output + input enable + other pin configuration
parameters (a set depends on real HW needs).

> Note that we added a enable-output for the same reason.
> See RZ/A1H HW Manual section "Table 54.7 Alternative Functions that PIPCn.PIPCnm Bit Should be Set to 0"

Perhaps needs to be revisited as well.

P.S. It looks like more and more software engineers are going far from
real hardware when developing drivers...

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v4 2/2] of: Add unit tests for applying overlays
From: Frank Rowand @ 2017-04-28 15:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A, Michal Marek,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kbuild
In-Reply-To: <CAMuHMdWLhLqjNgz6iUOY8LY1Jjn3-_iV2ncbOxYLCLZbf=Rz+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 04/28/17 04:25, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Wed, Apr 26, 2017 at 2:09 AM,  <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
>> Existing overlay unit tests examine individual pieces of the overlay
>> code.  The new tests target the entire process of applying an overlay.
>>
>> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>> ---
>>
>> There are checkpatch warnings.  I have reviewed them and feel they
>> can be ignored.
>>
>>  drivers/of/fdt.c                                 |  14 +-
>>  drivers/of/of_private.h                          |  12 +
>>  drivers/of/unittest-data/Makefile                |  17 +-
>>  drivers/of/unittest-data/overlay.dts             |  53 ++++
>>  drivers/of/unittest-data/overlay_bad_phandle.dts |  20 ++
>>  drivers/of/unittest-data/overlay_base.dts        |  80 ++++++
>>  drivers/of/unittest.c                            | 317 +++++++++++++++++++++++
>>  7 files changed, 505 insertions(+), 8 deletions(-)
>>
>>  create mode 100644 drivers/of/unittest-data/overlay.dts
>>  create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
>>  create mode 100644 drivers/of/unittest-data/overlay_base.dts
> 
> Shouldn't these be called .dtso instead of .dts?

That is a good question.  I'm not worried about solving it this week for
this patch, because this could turn into a bikeshed and I can always
fix it with a patch if we decide to change.  But if we do want to change
the naming, I would like to make the decision in the next couple of
months.  I would like to see more progress on overlays in general
this summer, and plan to be working on them myself.

I _think_ there has been some discussion about source file naming on the
devicetree-compiler or devicetree list in the far distant past.  Or I
may just be mis-remembering.

As far as I know, the current dtc does not know any suffixes other than
.dts for source files.  Not that the compiler has to know, we can always
specify '-I dts'.


> 
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
> 
>> +# enable creation of __symbols__ node
>> +DTC_FLAGS_overlay := -@
>> +DTC_FLAGS_overlay_bad_phandle := -@
>> +DTC_FLAGS_overlay_base := -@
> 
> This flag is needed for all DTs that will be involved with overlays.
> 
> Hence what about enabling this globally instead, cfr. "Enable DT symbols when"
> CONFIG_OF_OVERLAY is used
> ("http://www.spinics.net/lists/devicetree/msg103363.html")?

And another really good question.

There are some issues.  I have thought about it enough to know there are issues,
but do not have a solution and do not think I know all the issues.  Some
possible issues (or perceived issues) are:

- The size of __symbols__ in an FDT (akd compile .dtb image) in either a kernel
  image or a bootloader if overlays are not actually needed on a given system
  (even if the system is physically capable of using overlays).

- The size of __symbols__ in kernel memory if overlays are not actually needed
  on a given system (even if the system is physically capable of using overlays.)
  This could be possibly be enabled/disabled by a boot command, even if
  __symbols__ is in the FDT.

- A base FDT might want to have __symbols__ included with the expectation that
  overlays will be used in the future.  (The FDT might be built for the boot
  loader, then be stable for many kernel releases.)

- Should the creation of __symbols__ be a global switch, or should it be
  controlled on a per dtb basis?  Or a combination of both?

Again, I'm not worried about an immediate, this week solution, but I would
like to make good progress on this in the next couple of months.

-Frank

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

--
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 linux v9 2/5] hwmon: occ: Add sysfs interface
From: Eddie James @ 2017-04-28 15:22 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: devicetree, linux-kernel, linux-hwmon, linux-doc, jdelvare,
	corbet, mark.rutland, robh+dt, wsa, andrew, benh, joel,
	Edward A. James
In-Reply-To: <818c5a81-1791-aa0d-a3fe-e0976041cdc9@roeck-us.net>



On 04/02/2017 06:19 AM, Guenter Roeck wrote:
> On 03/14/2017 01:55 PM, Eddie James wrote:
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> Add a generic mechanism to expose the sensors provided by the OCC in
>> sysfs.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>>  Documentation/hwmon/occ       |  62 +++++++++++
>>  drivers/hwmon/occ/Makefile    |   2 +-
>>  drivers/hwmon/occ/occ_sysfs.c | 253 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/hwmon/occ/occ_sysfs.h |  25 +++++
>>  4 files changed, 341 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/hwmon/occ/occ_sysfs.c
>>  create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>>
>> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
>> index d1c863b..580af26 100644
>> --- a/Documentation/hwmon/occ
>> +++ b/Documentation/hwmon/occ
>> @@ -27,6 +27,68 @@ Currently, all versions of the OCC support four 
>> types of sensor data: power,
>>  temperature, frequency, and "caps," which indicate limits and 
>> thresholds used
>>  internally on the OCC.
>>
>> +sysfs Entries
>> +-------------
>> +
>> +The OCC driver uses the hwmon sysfs framework to provide data to 
>> userspace.
>> +
>> +The driver exports a number of sysfs files for each type of sensor. The
>> +sensor-specific files vary depending on the processor type, though 
>> many of the
>> +attributes are common for both the POWER8 and POWER9.
>> +
>> +The hwmon interface cannot define every type of sensor that may be 
>> used.
>> +Therefore, the frequency sensor on the OCC uses the "input" type 
>> sensor defined
>> +by the hwmon interface, rather than defining a new type of custom 
>> sensor.
>> +
>> +Below are detailed the names and meaning of each sensor file for 
>> both types of
>> +processors. All sensors are read-only unless otherwise specified. 
>> <x> indicates
>> +the hwmon index. sensor id indicates the unique internal OCC 
>> identifer. Please
>> +see the POWER OCC specification for details on all these sensor values.
>> +
>> +frequency:
>> +    all processors:
>> +        in<x>_input - frequency value
>> +        in<x>_label - sensor id
>> +temperature:
>> +    POWER8:
>> +        temp<x>_input - temperature value
>> +        temp<x>_label - sensor id
>> +    POWER9 (in addition to above):
>> +        temp<x>_type - FRU type
>> +
>> +power:
>> +    POWER8:
>> +        power<x>_input - power value
>> +        power<x>_label - sensor id
>> +        power<x>_average - accumulator
>> +        power<x>_average_interval - update tag (number of samples in
>> +            accumulator)
>> +    POWER9:
>> +        power<x>_input - power value
>> +        power<x>_label - sensor id
>> +        power<x>_average_min - accumulator[0]
>> +        power<x>_average_max - accumulator[1] (64 bits total)
>> +        power<x>_average_interval - update tag
>> +        power<x>_reset_history - (function_id | (apss_channel << 8)
>> +
>> +caps:
>> +    POWER8:
>> +        power<x>_cap - current powercap
>> +        power<x>_cap_max - max powercap
>> +        power<x>_cap_min - min powercap
>> +        power<x>_max - normal powercap
>> +        power<x>_alarm - user powercap, r/w
>> +    POWER9:
>> +        power<x>_cap_alarm - user powercap source
>> +
>> +The driver also provides two sysfs entries through hwmon to better
>> +control the driver and monitor the master OCC. Though there may be 
>> multiple
>> +OCCs present on the system, these two files are only present for the 
>> "master"
>> +OCC.
>> +    name - read the name of the driver
>> +    update_interval - read or write the minimum interval for polling 
>> the
>> +        OCC.
>> +
>>  BMC - Host Communications
>>  -------------------------
>>
>> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
>> index 3ed79a5..67b5367 100644
>> --- a/drivers/hwmon/occ/Makefile
>> +++ b/drivers/hwmon/occ/Makefile
>> @@ -1 +1 @@
>> -obj-$(CONFIG_SENSORS_IBM_OCC) += occ.o
>> +obj-$(CONFIG_SENSORS_IBM_OCC) += occ.o occ_sysfs.o
>> diff --git a/drivers/hwmon/occ/occ_sysfs.c 
>> b/drivers/hwmon/occ/occ_sysfs.c
>> new file mode 100644
>> index 0000000..50b20e2
>> --- /dev/null
>> +++ b/drivers/hwmon/occ/occ_sysfs.c
>> @@ -0,0 +1,253 @@
>> +/*
>> + * occ_sysfs.c - OCC sysfs interface
>> + *
>> + * This file contains the methods and data structures for 
>> implementing the OCC
>> + * hwmon sysfs entries.
>> + *
>> + * Copyright 2017 IBM Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/init.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/slab.h>
>> +#include "occ.h"
>> +#include "occ_sysfs.h"
>> +
>> +#define OCC_HWMON_NAME_LENGTH    32
>> +
>> +struct occ_sysfs {
>> +    struct device *dev;
>> +    struct occ *occ;
>> +
>> +    char label_buffer[OCC_HWMON_NAME_LENGTH + 1];
>> +    char hwmon_name[OCC_HWMON_NAME_LENGTH + 1];
>> +    const u32 *sensor_hwmon_configs;
>> +    struct hwmon_channel_info **occ_sensors;
>> +    struct hwmon_chip_info occ_info;
>> +    u16 user_powercap;
>> +};
>> +
>> +static int occ_hwmon_read(struct device *dev, enum 
>> hwmon_sensor_types type,
>> +              u32 attr, int channel, long *val)
>> +{
>> +    int rc;
>> +    struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +    struct occ *occ = driver->occ;
>> +
>> +    switch (type) {
>> +    case hwmon_in:
>> +        rc = occ_get_sensor_field(occ, FREQ, channel, attr, val);
>> +        break;
>> +    case hwmon_temp:
>> +        rc = occ_get_sensor_field(occ, TEMP, channel, attr, val);
>> +        break;
>> +    case hwmon_power:
>> +        rc = occ_get_sensor_field(occ, POWER, channel, attr, val);
>> +        break;
>> +    default:
>> +        rc = -EOPNOTSUPP;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +static int occ_hwmon_read_string(struct device *dev,
>> +                 enum hwmon_sensor_types type, u32 attr,
>> +                 int channel, const char **str)
>> +{
>> +    int rc;
>> +    unsigned long val = 0;
>> +    struct occ_sysfs *driver = dev_get_drvdata(dev);
>> +
>> +    if (!((type == hwmon_in && attr == hwmon_in_label) ||
>> +        (type == hwmon_temp && attr == hwmon_temp_label) ||
>> +        (type == hwmon_power && attr == hwmon_power_label)))
>> +        return -EOPNOTSUPP;
>> +
>> +    /* will fetch the "label", the sensor_id */
>> +    rc = occ_hwmon_read(dev, type, attr, channel, &val);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    /* just use one label buffer for all sensors. works with current 
>> hwmon
>> +     * implementation. only alternative is to store a buffer for each
>> +     * sensor, which gets expensive quickly.
>> +     */
>
> Sorry for the late reply.
>
> No, this doesn't work and is racy. Reading all labels from multiple 
> threads
> in parallel will result in random data.
>
> The label is supposed to be constant for each sensor. If it isn't, it 
> is not
> a label. Either create it as constant and use the generated string, or 
> drop
> the attribute.
>
> Guenter

Thanks for catching that. The driver is undergoing some significant 
restructuring as a result of changes in design to the low-level driver 
for the P9 side of things (actually doing the communication with the 
OCC). Depending on how extensive the changes are (haven't had time to 
finalize), this patchset could be abandoned and I'll start again.

Thanks for the feedback so far.

Eddie


^ permalink raw reply

* RE: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable
From: Chris Brandt @ 2017-04-28 15:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
	Rob Herring, Mark Rutland, Russell King - ARM Linux,
	Linux-Renesas, linux-gpio@vger.kernel.org, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75VeLtOuty=iaxuCaH43rnBZDTT03iH7JyjDJBQhTD-EjzA@mail.gmail.com>

On Friday, April 28, 2017, Andy Shevchenko wrote:
> Had you read the following, esp. Note there?
> 
> * @PIN_CONFIG_INPUT_ENABLE: enable the pin's input.  Note that this does
> not
> *      affect the pin's ability to drive output.  1 enables input, 0
> disables
> *      input.
> 
> For me manual is clearly tells about this settings:
> "This register enables or disables the input buffer while the output
> buffer is enabled."


But, then if we use "input-enable" to get bi-directional functionality, now we need something to replace what we were using "input-enable" for.
We were using "input-enable" to signal when the pin function that we set also needs to be forcible set to input by the software (once again, because the HW is not smart enough to do it on its own), but is different than the bi-directional functionality (ie, a different register setting).


So, if we replace "bi-directional" with "input-enable" (since logically internally that is what is going on), what do we use for the special pins that the HW manual says "hey, you need to manually set these pins to input with SW because the pin selection HW can't do it correctly)". Note that we added a enable-output for the same reason.
See RZ/A1H HW Manual section "Table 54.7 Alternative Functions that PIPCn.PIPCnm Bit Should be Set to 0"


Chris

^ permalink raw reply

* RE: [PATCH v8 1/3] backlight arcxcnn add arc to vendor prefix
From: Olimpiu Dejeu @ 2017-04-28 15:04 UTC (permalink / raw)
  To: 'Jingoo Han'
  Cc: 'Rob Herring', 'Lee Jones',
	'Geert Uytterhoeven', linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	'Linux Fbdev development list',
	devicetree-u79uwXL29TY76Z2rM5mHXA, Brian Dodge,
	'Joe Perches', 'Daniel Thompson'
In-Reply-To: <001301d2bf7d$919d2d60$b4d78820$@gmail.com>

> On Thursday, April 27, 2017 8:37 AM, Geert Uytterhoeven wrote:
> > On Tue, Apr 25, 2017 at 6:36 PM, Jingoo Han <jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > On Monday, April 24, 2017 1:56 PM, Olimpiu Dejeu wrote:
> > >>
> > >> On Mon, April 24, 2017 11:10 AM, Rob Herring < robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > >>
> > >> > On Wed, Mar 15, 2017 at 2:45 PM, Olimpiu Dejeu
> > <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> > >> wrote:
> > >> >> backlight: Add arc to vendor prefixes
> > >> >> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> > >> >> ---
> > >> >> v8:
> > >> >> - Version to match other patches in set
> > >> >>
> > >> >>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > >> >>  1 file changed, 1 insertion(+)
> > >> >>
> > >> >> diff --git 
> > >> >> a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > >> >> b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > >> >> index 16d3b5e..6f33a4b 100644
> > >> >> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> > >> >> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> > >> >> @@ -28,6 +28,7 @@ andestech     Andes Technology Corporation
> > >> >>  apm    Applied Micro Circuits Corporation (APM)
> > >> >>  aptina Aptina Imaging
> > >> >>  arasan Arasan Chip Systems
> > >> >> +arc    Arctic Sand
> > >>
> > >> >arc is also a cpu arch. While not a vendor, it could be confusing. 
> > >> >How
> > >> about "arctic" >instead?
> > >>
> > >> Rob, will do, i.e. I will change it to "arctic"
> > >
> > > Hi Olimpiu,
> > >
> > > Oh, "arc" and "arctic" is totally different.
> > > In my opinion, one of the purposes of DT is to describe hardware stuffs.
> > > So, please use more detailed words.
> > 
> > Already acquired by Murata?

> Yes, Murata already announced the agreement of acquisition. [1]

> To Olimpiu Dejeu,
> What is your company's plan about naming?
> The brand name 'Artic Sand' will be continued or not?

The brand name 'Arctic Sand' will be continued 

> If not, you should replace 'Artic Sand' with 'Murata'.
> As you said earlier, changing DT names is not easy after merging.

> Best regards,
> Jingoo Han


--
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] clk: stm32h7: Add stm32h743 clock driver
From: Gabriel Fernandez @ 2017-04-28 14:56 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
	Alexandre Torgue, Michael Turquette, Nicolas Pitre, Arnd Bergmann,
	daniel.thompson, andrea.merello, radoslaw.pietrzyk, Lee Jones,
	devicetree, linux-arm-kernel, linux-kernel, linux-clk,
	ludovic.barre, olivier.bideau, amelie.delaunay
In-Reply-To: <20170407195152.GH7065@codeaurora.org>

Hi Stephen

Sorry for delay i was on sick live

On 04/07/2017 09:51 PM, Stephen Boyd wrote:
> On 04/06, Gabriel Fernandez wrote:
>> On 04/06/2017 12:32 AM, Stephen Boyd wrote:
>>> On 03/15, gabriel.fernandez@st.com wrote:
>>>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>>>> new file mode 100644
>>>> index 0000000..9d4b587
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>>>> @@ -0,0 +1,152 @@
>>>> +
>>>> +	rcc: rcc@58024400 {
>>>> +		#reset-cells = <1>;
>>>> +		#clock-cells = <2>
>>>> +		compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>>>> +		reg = <0x58024400 0x400>;
>>>> +		clocks = <&clk_hse>, <&clk_lse>, <&clk_i2s_ckin>;
>>>> +
>>>> +		st,syscfg = <&pwrcfg>;
>>>> +
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <0>;
>>>> +
>>>> +		vco1@58024430 {
>>>> +			#clock-cells = <0>;
>>>> +			compatible = "stm32,pll";
>>>> +			reg = <0>;
>>> reg is super confusing and doesn't match unit address.
>> ok i fixed it in the v2
>>
>>>> +		};
>>> Why? Shouldn't we know this from the compatible string how many
>>> PLLs there are and where they're located? Export the PLLs through
>>> rcc node's clock-cells?
>>>
>> Because i need to offer the possibility to change the PLL VCO
>> frequencies at the start-up of this driver clock.
>> The VCO algorithm needs a division factor, a multiplication factor
>> and a fractional factor.
>> Lot's of solution are possible for one frequency and it's nightmare
>> to satisfy the 3 output dividers of the PLL.
> Sure, but do we need to configure that on a per-board basis or a
> per-SoC basis? If it's just some configuration, I wonder why we
> don't put that into the driver and base it off some compatible
> string that includes the SoC the device is for.
>
I prefer to let in first, the responsibility of the boot loader to 
change VCO parameters.
Then i propose a new version without DT configuration of PLL's
are you ok for that ?

best regards

Gabriel



To simply



^ permalink raw reply

* RE: [PATCH v2] clk: Provide dummy of_clk_get_from_provider() for compile-testing
From: Langer, Thomas @ 2017-04-28 14:55 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Turquette, Stephen Boyd, Russell King,
	Rob Herring, Mark Rutland, John Crispin, Ralf Baechle
  Cc: linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
In-Reply-To: <1493384933-31297-1-git-send-email-geert+renesas@glider.be>



> -----Original Message-----
> From: devicetree-owner@vger.kernel.org [mailto:devicetree-
> owner@vger.kernel.org] On Behalf Of Geert Uytterhoeven
> Sent: Friday, April 28, 2017 3:09 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@codeaurora.org>; Russell King <linux@armlinux.org.uk>; Rob Herring
> <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; John Crispin
> <john@phrozen.org>; Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-
> mips@linux-mips.org; linux-kernel@vger.kernel.org; Geert Uytterhoeven
> <geert+renesas@glider.be>
> Subject: [PATCH v2] clk: Provide dummy of_clk_get_from_provider() for
> compile-testing
> 
> When CONFIG_ON=n, dummies are provided for of_clk_get() and
> of_clk_get_by_name(), but not for of_clk_get_from_provider().
> 
> Provide a dummy for the latter, to improve the ability to do
> compile-testing.  This requires removing the existing dummy in the
> Lantiq clock code.
> 
> Fixes: 766e6a4ec602d0c1 ("clk: add DT clock binding support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Regarding the Lantiq part:
Acked-by: Thomas Langer <thomas.langer@intel.com>

> ---
> v2:
>   - Remove conflicting dummy in Lantiq clock code (reported by 0day).
> ---
>  arch/mips/lantiq/clk.c | 5 -----
>  include/linux/clk.h    | 4 ++++
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/lantiq/clk.c b/arch/mips/lantiq/clk.c
> index 149f0513c4f5d0d4..a263d1b751ffe48d 100644
> --- a/arch/mips/lantiq/clk.c
> +++ b/arch/mips/lantiq/clk.c
> @@ -160,11 +160,6 @@ void clk_deactivate(struct clk *clk)
>  }
>  EXPORT_SYMBOL(clk_deactivate);
> 
> -struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
> -{
> -	return NULL;
> -}
> -
>  static inline u32 get_counter_resolution(void)
>  {
>  	u32 res;
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index e9d36b3e49de5b1b..3ed97abb5cbb7f94 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -539,6 +539,10 @@ static inline struct clk *of_clk_get_by_name(struct
> device_node *np,
>  {
>  	return ERR_PTR(-ENOENT);
>  }
> +static inline struct clk *of_clk_get_from_provider(struct of_phandle_args
> *clkspec)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
>  #endif
> 
>  #endif
> --
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" 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 v5 01/10] pinctrl: generic: Add bi-directional and output-enable
From: Andy Shevchenko @ 2017-04-28 14:53 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Linus Walleij, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
	Rob Herring, Mark Rutland, Russell King - ARM Linux,
	Linux-Renesas, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <SG2PR06MB11658EC3A3A80C57E6F539AC8A130-ESzmfEwOt/xoAsOJh7vwSm0DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

On Fri, Apr 28, 2017 at 4:18 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> On Friday, April 28, 2017, Andy Shevchenko wrote:
>> > In the RZ/A1 HW manual you can kind of see that in 54.18 Port Control
>> Logical Diagram (but that wasn't obvious to me at first).
>>
>> Please, post a link to it or copy essential parts.

> The schematic is included in the "User's manual"
> https://www.renesas.com/en-us/doc/products/tool/doc/003/r20ut2596ej_r7s72100evum.pdf

Not that one I would like to see...

> The RZ/A1H Hardware manual is here:
> https://www.renesas.com/en-us/document/hw-manual?hwLayerShowFlg=true&prdLayerId=186374&layerName=RZ%252FA1H&coronrService=document-prd-search&hwDocUrl=%2Fen-us%2Fdoc%2Fproducts%2Fmpumcu%2Fdoc%2Frz%2Fr01uh0403ej0300_rz_a1h.pdf&hashKey=54f335753742b5add524d4725b7242e6
>
> Chapter 54 is the port/pin controller.
>
> "54.18 Port Control Logical Diagram" is the diagram I was talking about. Note that is says "Note: This figure shows the logic for reference, not the circuit."
>
> "54.3.13 Port Bidirection Control Register (PBDCn)" is the magic register needed to get some pins to work.

This is useful. Thanks.

>> I'm quite skeptical  that cheap hardware can implement something more
>> costable than simplest open-source / open-drain + bias
>
> I don't think this is an open-source / open-drain + bias issue. It's a "the internal signal paths are not getting hooked up correctly" issue.

Had you read the following, esp. Note there?

* @PIN_CONFIG_INPUT_ENABLE: enable the pin's input.  Note that this does not
*      affect the pin's ability to drive output.  1 enables input, 0 disables
*      input.

For me manual is clearly tells about this settings:
"This register enables or disables the input buffer while the output
buffer is enabled."

> Regardless, on this part, we needed a way to flag that some pins when put in some function modes needed 'an extra register setting'. At first we tried to sneak that info in with a simple #define in the pin/pinmux DT node properties. But, Linus didn't want it there so we had to make up a new generic property called "bi-directional".
>
> What is your end goal here? Get "bi-directional" changed to something else?

My goal is to reduce amount of (useless) entities. See Occam's razor
for details.

Linus, for me it looks like better to revert that change, until we
will have clear picture why existing configuration parameters can't
work.

-- 
With Best Regards,
Andy Shevchenko
--
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 10/10] arm: dts: genmai: Add ethernet pin group
From: Chris Brandt @ 2017-04-28 14:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jacopo Mondi, Linus Walleij, Geert Uytterhoeven, Laurent Pinchart,
	Rob Herring, Mark Rutland, Russell King, Linux-Renesas,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdU2CvipQQQ8-wxsiniHZJQmUMHv-t52Pw=sHD3YTd7Yug-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Geert,

On Friday, April 28, 2017, Geert Uytterhoeven wrote:
> >> Shouldn't the interrupt (connected to P1_15) be described?
> >
> > That interrupt pin from the PHY is not used. It did not need to be
> connected.
> 
> But it is connected, according to the schematics.

P1_15 can be configured as:
 * GPIO_IN
 * AN7
 * AVB_CAPTURE

So...I guess you would 'describe' it as an GPIO-input.


> DT describes hardware, not software limitations.

Describing things is fine, but the kernel code takes the DT and then start configuring things based on it.

For example, on the RSK board, that line is connected to P4_14. P4_14 can be configured as IRQ6...but IRQ6 comes out in 8 different pin choices, and I might want to use one of those other choices, so I don't want to describe P4_14 as an interrupt.

If it was just describing that "pin 15 of the PHY chip is tied to pin Y19 of the RZ/A1H", that's fine because it's hardware connection that's not going to change.
But...the DT also defines the pin muxing...which is a software decision (do I want to get interrupt or just manually poll or simple ignore it).
This is the part of the whole "DT is for hardware description only" that doesn't really make sense to me.


Chris


^ permalink raw reply

* Re: [PATCH] ARM: dts: r7s72100: add usb clocks to device tree
From: Geert Uytterhoeven @ 2017-04-28 14:27 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <SG2PR06MB1165D398BD7100FAC5AAEDC68A130-ESzmfEwOt/xoAsOJh7vwSm0DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

Hi Chris,

On Fri, Apr 28, 2017 at 2:47 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> On Friday, April 28, 2017, Simon Horman wrote:
>> On Fri, Apr 28, 2017 at 09:34:54AM +0200, Geert Uytterhoeven wrote:
>> > On Thu, Apr 27, 2017 at 9:10 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> wrote:
>> > > This adds the USB0 and USB1 clocks to the device tree.

>> > Simon: see Section 29.3.2 (BUSWAIT) for the reference to the P1 clock.
>>
>> Thanks, I see that now.
>
> I was going to say that I simply look at sections:
>   6.10.6 Internal Clock Signals (1)
>   6.10.7 Internal Clock Signals (2)
>
> Because it lists all the IP blocks and their corresponding clock sources in one place.

Cool, I grew up with rev. 0.60 of the user manual, so I wasn't aware of
that secton.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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/8] [RFC] dt-bindings: renesas: Document R-Car H3 and M3-W SiP DT bindings
From: Geert Uytterhoeven @ 2017-04-28 14:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: Geert Uytterhoeven, Simon Horman, Magnus Damm, Kuninori Morimoto,
	Yoshihiro Shimoda, Mark Rutland, Linux-Renesas,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170428133923.ycbqixcxvcsonocn@rob-hp-laptop>

Hi Rob,

On Fri, Apr 28, 2017 at 3:39 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Apr 19, 2017 at 11:15:44AM +0200, Geert Uytterhoeven wrote:
>> Document the SiP ("System-in-Package") versions of the R-Car H3 and M3-W
>> SoCs, which contain an R-Car H3 or M3-W SoC, RAM, and HyperFlash.
>>
>> Add their compatible values to all boards equipped with R-Car Gen3 SiPs.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Questions:
>>   - Do we need more compatible values, for different configurations?
>>     At least r8j7796 is available with either 2 GiB or 4 GiB of RAM,
>>     possibly using RAM parts from different vendors.
>
> Same die, just a different package? If so, I don't think you need a
> different compatible. It's going to be a different board from any
> non-SiP which should be enough to distinguish.

An SiP is more like a CPU daughterboard.

The different SiP-versions based on r8a7795 contain the same r8a7795 SoC die,
but different amounts of RAM, i.e. different RAM dies.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 2/2] [media] platform: add video-multiplexer subdevice driver
From: Philipp Zabel @ 2017-04-28 14:13 UTC (permalink / raw)
  To: linux-media
  Cc: devicetree, Steve Longerbeam, Peter Rosin, Sakari Ailus,
	Pavel Machek, Rob Herring, Mark Rutland, Vladimir Zapolskiy,
	kernel, Philipp Zabel, Sascha Hauer, Steve Longerbeam
In-Reply-To: <20170428141330.16187-1-p.zabel@pengutronix.de>

This driver can handle SoC internal and external video bus multiplexers,
controlled by mux controllers provided by the mux controller framework,
such as MMIO register bitfields or GPIOs. The subdevice passes through
the mbus configuration of the active input to the output side.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
This has been last sent as part of the i.MX media series.

Changes since https://patchwork.kernel.org/patch/9647869/:
 - Split out the actual mux operation to be provided by the mux controller
   framework [1]. GPIO and MMIO control can be provided by individual mux
   controller drivers [2][3].
   [1] https://patchwork.kernel.org/patch/9695837/
   [2] https://patchwork.kernel.org/patch/9695839/
   [3] https://patchwork.kernel.org/patch/9704509/
 - Shortened 'video-multiplexer' to 'video-mux', replaced all instances of
   vidsw with video_mux.
 - Made the mux inactive by default, only activated by user interaction.
 - Added CONFIG_OF and CONFIG_MULTIPLEXER dependencies.
 - Reuse subdev.entity.num_pads instead of keeping our own count.
 - Removed implicit link disabling. Instead, trying to enable a second
   sink pad link yields -EBUSY.
 - Merged _async_init into _probe.
 - Removed superfluous pad index check from _set_format.
 - Added is_source_pad helper to tell source and sink pads apart.
 - Removed test for status property in endpoint nodes. Disable the remote
   device or sever the endpoint link to disable a sink pad.
---
 drivers/media/platform/Kconfig     |   6 +
 drivers/media/platform/Makefile    |   2 +
 drivers/media/platform/video-mux.c | 341 +++++++++++++++++++++++++++++++++++++
 3 files changed, 349 insertions(+)
 create mode 100644 drivers/media/platform/video-mux.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index c9106e105baba..b046a6d39fee5 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -74,6 +74,12 @@ config VIDEO_M32R_AR_M64278
 	  To compile this driver as a module, choose M here: the
 	  module will be called arv.
 
+config VIDEO_MUX
+	tristate "Video Multiplexer"
+	depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER && MULTIPLEXER
+	help
+	  This driver provides support for N:1 video bus multiplexers.
+
 config VIDEO_OMAP3
 	tristate "OMAP 3 Camera support"
 	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 349ddf6a69da2..fd2735ca3ff75 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -27,6 +27,8 @@ obj-$(CONFIG_VIDEO_SH_VEU)		+= sh_veu.o
 
 obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)	+= m2m-deinterlace.o
 
+obj-$(CONFIG_VIDEO_MUX)			+= video-mux.o
+
 obj-$(CONFIG_VIDEO_S3C_CAMIF) 		+= s3c-camif/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) 	+= exynos4-is/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)	+= s5p-jpeg/
diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
new file mode 100644
index 0000000000000..419541729f67e
--- /dev/null
+++ b/drivers/media/platform/video-mux.c
@@ -0,0 +1,341 @@
+/*
+ * video stream multiplexer controlled via mux control
+ *
+ * Copyright (C) 2013 Pengutronix, Sascha Hauer <kernel@pengutronix.de>
+ * Copyright (C) 2016 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+#include <media/v4l2-of.h>
+
+struct video_mux {
+	struct v4l2_subdev subdev;
+	struct media_pad *pads;
+	struct v4l2_mbus_framefmt *format_mbus;
+	struct v4l2_of_endpoint *endpoint;
+	struct mux_control *mux;
+	int active;
+};
+
+static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev *sd)
+{
+	return container_of(sd, struct video_mux, subdev);
+}
+
+static inline bool is_source_pad(struct video_mux *vmux, unsigned int pad)
+{
+	return pad == vmux->subdev.entity.num_pads - 1;
+}
+
+static int video_mux_link_setup(struct media_entity *entity,
+				const struct media_pad *local,
+				const struct media_pad *remote, u32 flags)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+	int ret;
+
+	/*
+	 * The mux state is determined by the enabled sink pad link.
+	 * Enabling or disabling the source pad link has no effect.
+	 */
+	if (is_source_pad(vmux, local->index))
+		return 0;
+
+	dev_dbg(sd->dev, "link setup '%s':%d->'%s':%d[%d]",
+		remote->entity->name, remote->index, local->entity->name,
+		local->index, flags & MEDIA_LNK_FL_ENABLED);
+
+	if (flags & MEDIA_LNK_FL_ENABLED) {
+		if (vmux->active == local->index)
+			return 0;
+
+		if (vmux->active >= 0)
+			return -EBUSY;
+
+		dev_dbg(sd->dev, "setting %d active\n", local->index);
+		ret = mux_control_try_select(vmux->mux, local->index);
+		if (ret < 0)
+			return ret;
+		vmux->active = local->index;
+	} else {
+		if (vmux->active != local->index)
+			return 0;
+
+		dev_dbg(sd->dev, "going inactive\n");
+		mux_control_deselect(vmux->mux);
+		vmux->active = -1;
+	}
+
+	return 0;
+}
+
+static struct media_entity_operations video_mux_ops = {
+	.link_setup = video_mux_link_setup,
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static bool video_mux_endpoint_disabled(struct device_node *ep)
+{
+	struct device_node *rpp = of_graph_get_remote_port_parent(ep);
+
+	return !of_device_is_available(rpp);
+}
+
+static int video_mux_g_mbus_config(struct v4l2_subdev *sd,
+				   struct v4l2_mbus_config *cfg)
+{
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+	struct v4l2_of_endpoint *endpoint;
+	struct media_pad *pad;
+	int ret;
+
+	if (vmux->active == -1) {
+		dev_err(sd->dev, "no configuration for inactive mux\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Retrieve media bus configuration from the entity connected to the
+	 * active input
+	 */
+	pad = media_entity_remote_pad(&vmux->pads[vmux->active]);
+	if (pad) {
+		sd = media_entity_to_v4l2_subdev(pad->entity);
+		ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
+		if (ret == -ENOIOCTLCMD)
+			pad = NULL;
+		else if (ret < 0) {
+			dev_err(sd->dev, "failed to get source configuration\n");
+			return ret;
+		}
+	}
+	if (!pad) {
+		endpoint = &vmux->endpoint[vmux->active];
+
+		/* Mirror the input side on the output side */
+		cfg->type = endpoint->bus_type;
+		if (cfg->type == V4L2_MBUS_PARALLEL ||
+		    cfg->type == V4L2_MBUS_BT656)
+			cfg->flags = endpoint->bus.parallel.flags;
+	}
+
+	return 0;
+}
+
+static int video_mux_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+	struct v4l2_subdev *upstream_sd;
+	struct media_pad *pad;
+
+	if (vmux->active == -1) {
+		dev_err(sd->dev, "Can not start streaming on inactive mux\n");
+		return -EINVAL;
+	}
+
+	pad = media_entity_remote_pad(&sd->entity.pads[vmux->active]);
+	if (!pad) {
+		dev_err(sd->dev, "Failed to find remote source pad\n");
+		return -ENOLINK;
+	}
+
+	if (!is_media_entity_v4l2_subdev(pad->entity)) {
+		dev_err(sd->dev, "Upstream entity is not a v4l2 subdev\n");
+		return -ENODEV;
+	}
+
+	upstream_sd = media_entity_to_v4l2_subdev(pad->entity);
+
+	return v4l2_subdev_call(upstream_sd, video, s_stream, enable);
+}
+
+static const struct v4l2_subdev_video_ops video_mux_subdev_video_ops = {
+	.g_mbus_config = video_mux_g_mbus_config,
+	.s_stream = video_mux_s_stream,
+};
+
+static struct v4l2_mbus_framefmt *
+__video_mux_get_pad_format(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_pad_config *cfg,
+			   unsigned int pad, u32 which)
+{
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+
+	switch (which) {
+	case V4L2_SUBDEV_FORMAT_TRY:
+		return v4l2_subdev_get_try_format(sd, cfg, pad);
+	case V4L2_SUBDEV_FORMAT_ACTIVE:
+		return &vmux->format_mbus[pad];
+	default:
+		return NULL;
+	}
+}
+
+static int video_mux_get_format(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_pad_config *cfg,
+			    struct v4l2_subdev_format *sdformat)
+{
+	sdformat->format = *__video_mux_get_pad_format(sd, cfg, sdformat->pad,
+						   sdformat->which);
+	return 0;
+}
+
+static int video_mux_set_format(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_pad_config *cfg,
+			    struct v4l2_subdev_format *sdformat)
+{
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+	struct v4l2_mbus_framefmt *mbusformat;
+
+	mbusformat = __video_mux_get_pad_format(sd, cfg, sdformat->pad,
+					    sdformat->which);
+	if (!mbusformat)
+		return -EINVAL;
+
+	/* Source pad mirrors active sink pad, no limitations on sink pads */
+	if (is_source_pad(vmux, sdformat->pad) && vmux->active >= 0)
+		sdformat->format = vmux->format_mbus[vmux->active];
+
+	*mbusformat = sdformat->format;
+
+	return 0;
+}
+
+static struct v4l2_subdev_pad_ops video_mux_pad_ops = {
+	.get_fmt = video_mux_get_format,
+	.set_fmt = video_mux_set_format,
+};
+
+static struct v4l2_subdev_ops video_mux_subdev_ops = {
+	.pad = &video_mux_pad_ops,
+	.video = &video_mux_subdev_video_ops,
+};
+
+static int video_mux_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct v4l2_of_endpoint endpoint;
+	struct device_node *ep;
+	struct video_mux *vmux;
+	unsigned int num_pads = 0;
+	int ret;
+	int i;
+
+	vmux = devm_kzalloc(dev, sizeof(*vmux), GFP_KERNEL);
+	if (!vmux)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, vmux);
+
+	v4l2_subdev_init(&vmux->subdev, &video_mux_subdev_ops);
+	snprintf(vmux->subdev.name, sizeof(vmux->subdev.name), "%s", np->name);
+	vmux->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	vmux->subdev.dev = dev;
+
+	/*
+	 * The largest numbered port is the output port. It determines
+	 * total number of pads.
+	 */
+	for_each_endpoint_of_node(np, ep) {
+		of_graph_parse_endpoint(ep, &endpoint.base);
+		num_pads = max(num_pads, endpoint.base.port + 1);
+	}
+
+	if (num_pads < 2) {
+		dev_err(dev, "Not enough ports %d\n", num_pads);
+		return -EINVAL;
+	}
+
+	vmux->mux = devm_mux_control_get(dev, NULL);
+	if (IS_ERR(vmux->mux)) {
+		ret = PTR_ERR(vmux->mux);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get mux: %d\n", ret);
+		return ret;
+	}
+
+	vmux->active = -1;
+	vmux->pads = devm_kzalloc(dev, sizeof(*vmux->pads) * num_pads,
+				  GFP_KERNEL);
+	vmux->format_mbus = devm_kzalloc(dev, sizeof(*vmux->format_mbus) *
+					 num_pads, GFP_KERNEL);
+	vmux->endpoint = devm_kzalloc(dev, sizeof(*vmux->endpoint) *
+				      (num_pads - 1), GFP_KERNEL);
+
+	for (i = 0; i < num_pads - 1; i++)
+		vmux->pads[i].flags = MEDIA_PAD_FL_SINK;
+	vmux->pads[num_pads - 1].flags = MEDIA_PAD_FL_SOURCE;
+
+	vmux->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
+	ret = media_entity_pads_init(&vmux->subdev.entity, num_pads,
+				     vmux->pads);
+	if (ret < 0)
+		return ret;
+
+	vmux->subdev.entity.ops = &video_mux_ops;
+
+	for_each_endpoint_of_node(np, ep) {
+		v4l2_of_parse_endpoint(ep, &endpoint);
+
+		if (video_mux_endpoint_disabled(ep)) {
+			dev_dbg(dev, "port %d disabled\n", endpoint.base.port);
+			continue;
+		}
+
+		vmux->endpoint[endpoint.base.port] = endpoint;
+	}
+
+	return v4l2_async_register_subdev(&vmux->subdev);
+}
+
+static int video_mux_remove(struct platform_device *pdev)
+{
+	struct video_mux *vmux = platform_get_drvdata(pdev);
+	struct v4l2_subdev *sd = &vmux->subdev;
+
+	v4l2_async_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+
+	return 0;
+}
+
+static const struct of_device_id video_mux_dt_ids[] = {
+	{ .compatible = "video-mux", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, video_mux_dt_ids);
+
+static struct platform_driver video_mux_driver = {
+	.probe		= video_mux_probe,
+	.remove		= video_mux_remove,
+	.driver		= {
+		.of_match_table = video_mux_dt_ids,
+		.name = "video-mux",
+	},
+};
+
+module_platform_driver(video_mux_driver);
+
+MODULE_DESCRIPTION("video stream multiplexer");
+MODULE_AUTHOR("Sascha Hauer, Pengutronix");
+MODULE_AUTHOR("Philipp Zabel, Pengutronix");
+MODULE_LICENSE("GPL");
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/2] [media] dt-bindings: Add bindings for video-multiplexer device
From: Philipp Zabel @ 2017-04-28 14:13 UTC (permalink / raw)
  To: linux-media
  Cc: devicetree, Steve Longerbeam, Peter Rosin, Sakari Ailus,
	Pavel Machek, Rob Herring, Mark Rutland, Vladimir Zapolskiy,
	kernel, Philipp Zabel, Sascha Hauer, Steve Longerbeam

Add bindings documentation for the video multiplexer device.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
This has been last sent as part of Steve's i.MX media series. Since the binding
changed, I've dropped Rob's ack.

Changes since https://patchwork.kernel.org/patch/9647951/:
 - Replaced re, bit-mask/shift, and gpios properties with a single mux-controls
   property, leaving the actual operation of the mux to a separate mux
   controller, as described by Peter's mux-controller bindings:
   https://patchwork.kernel.org/patch/9695835/
 - Shortened 'video-multiplexer' compatible to 'video-mux', aligning with the
   other mux bindings.
 - Added a comment about the optional ports node and a link to the OF graph
   bindings document.
---
 .../devicetree/bindings/media/video-mux.txt        | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/video-mux.txt

diff --git a/Documentation/devicetree/bindings/media/video-mux.txt b/Documentation/devicetree/bindings/media/video-mux.txt
new file mode 100644
index 0000000000000..63b9dc913e456
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-mux.txt
@@ -0,0 +1,60 @@
+Video Multiplexer
+=================
+
+Video multiplexers allow to select between multiple input ports. Video received
+on the active input port is passed through to the output port. Muxes described
+by this binding are controlled by a multiplexer controller that is described by
+the bindings in Documentation/devicetree/bindings/mux/mux-controller.txt
+
+Required properties:
+- compatible : should be "video-mux"
+- mux-controls : mux controller node to use for operating the mux
+- #address-cells: should be <1>
+- #size-cells: should be <0>
+- port@*: at least three port nodes containing endpoints connecting to the
+  source and sink devices according to of_graph bindings. The last port is
+  the output port, all others are inputs.
+
+Optionally, #address-cells, #size-cells, and port nodes can be grouped under a
+ports node as described in Documentation/devicetree/bindings/graph.txt.
+
+Example:
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>;
+	};
+
+	video-mux {
+		compatible = "video-mux";
+		mux-controls = <&mux>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			mux_in0: endpoint {
+				remote-endpoint = <&video_source0_out>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			mux_in1: endpoint {
+				remote-endpoint = <&video_source1_out>;
+			};
+		};
+
+		port@2 {
+			reg = <2>;
+
+			mux_out: endpoint {
+				remote-endpoint = <&capture_interface_in>;
+			};
+		};
+	};
+};
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] iio: adc: add driver for the ti-adc084s021 chip
From: Rob Herring @ 2017-04-28 13:52 UTC (permalink / raw)
  To: Mårten Lindahl
  Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
In-Reply-To: <1492786703-5149-1-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>

On Fri, Apr 21, 2017 at 04:58:23PM +0200, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> 
> This adds support for the Texas Instruments ADC084S021 ADC chip.
> 
> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/adc/ti-adc084s021.txt  |  25 ++

It's preferred to put bindings in a separate patch.

>  drivers/iio/adc/Kconfig                            |  12 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/ti-adc084s021.c                    | 342 +++++++++++++++++++++
>  4 files changed, 380 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
>  create mode 100644 drivers/iio/adc/ti-adc084s021.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
> new file mode 100644
> index 0000000..921eb46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
> @@ -0,0 +1,25 @@
> +* Texas Instruments' ADC084S021
> +
> +Required properties:
> + - compatible        : Must be "ti,adc084s021"
> + - reg               : SPI chip select number for the device
> + - vref-supply       : The regulator supply for ADC reference voltage
> + - spi-max-frequency : Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt
> +
> +Optional properties:
> + - spi-cpol          : SPI inverse clock polarity, as per spi-bus bindings
> + - spi-cpha          : SPI shifted clock phase (CPHA), as per spi-bus bindings
> + - spi-cs-high       : SPI chip select active high, as per spi-bus bindings

How are these optional? A given device should have specific properties 
required here.

Also, no need to define them, "per spi-bus bindings" is enough.

Rob
--
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 10/10] arm: dts: genmai: Add ethernet pin group
From: Chris Brandt @ 2017-04-28 13:50 UTC (permalink / raw)
  To: Linus Walleij, Jacopo Mondi
  Cc: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland,
	Russell King, Linux-Renesas, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdaiDMxaGM=vqeqVQ-6i3ecYhc+pudy4yt4zAZW-XZ4vCw@mail.gmail.com>

On Friday, April 28, 2017, Linus Walleij wrote:
> > Add pin configuration subnode for ETHER ethernet controller.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> (...)
> > +               pins_bidir {
> > +                       pinmux = <RZA1_PINMUX(3, 3, 2)>;/* P3_3 =
> ET_MDIO  */
> > +                       bi-directional;
> > +               };
> 
> So I'm against merging this until someone explains what "bi-directional"
> actually means, electrically speaking. What happens physically on this
> pin?
> 
> I think this just means open drain.
> 
> It is dangerous to merge things we don't understand.
> 
> Surely someone inside Renesas can answer this question.

I don't think this has anything to do with open drain because you need it for any pin that the peripheral IP block needs to transmit and receive over the same line, regardless of if it's a SDHI, I2C, Ethernet MDIO, etc...
It's more about of allowing the internal IP block signals to get hooked up to the IO pad signals.

Chris


^ permalink raw reply

* Re: [PATCH v2 1/9] dt-bindings: display: sun4i: Add component endpoint ID numbering scheme
From: Rob Herring @ 2017-04-28 13:48 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Mark Rutland, devicetree, linux-kernel, dri-devel, linux-sunxi,
	Maxime Ripard, linux-arm-kernel
In-Reply-To: <20170421083857.29636-2-wens@csie.org>

On Fri, Apr 21, 2017 at 04:38:49PM +0800, Chen-Yu Tsai wrote:
> The Allwinner display pipeline contains many hardware components, some
> of which can consume data from one of multiple upstream components.
> The numbering scheme of these components must be encoded into the device
> tree so the driver can figure out which component out of two or more of
> the same type it is supposed to use or program.
> 
> This patch adds the constraint that local endpoint IDs must be the index
> or number of the remote endpoint's hardware block, for all components
> in the display pipeline up to the TCONs.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 57a8d0610062..7acdbf14ae1c 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -4,6 +4,16 @@ Allwinner A10 Display Pipeline
>  The Allwinner A10 Display pipeline is composed of several components
>  that are going to be documented below:
>  
> +For the input port of all components up to the TCON in the display
> +pipeline, if there are multiple components, the local endpoint IDs
> +must correspond to the index of the upstream block. For example, if
> +the remote endpoint is Frontend 1, then the local endpoint ID must
> +be 1.
> +
> +Conversely, for the output ports of the same group, the remote endpoint
> +ID must be the index of the local hardware block. If the local backend
> +is backend 1, then the remote endpoint ID must be 1.

It would be clearer if you just explicitly listed IDs and their 
connections. From how this is worded, it would not work if you had 
connections like this:

DevA 0
DevA 1
DevB 0
DevB 1

These would need to be endpoints 0-3 in TCON, and that doesn't reflect 
the remote devices' index.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] iio: adc: Drop if clock from Renesas GyroADC bindings
From: Rob Herring @ 2017-04-28 13:42 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-renesas-soc, devicetree, Marek Vasut, Geert Uytterhoeven,
	Jonathan Cameron
In-Reply-To: <20170420154216.32709-1-marek.vasut+renesas@gmail.com>

On Thu, Apr 20, 2017 at 05:42:16PM +0200, Marek Vasut wrote:
> The "if" interface clock speed is actually derived from the "fck"
> block clock, as in the hardware they are the same clock. Drop the
> incorrect second "if" clock and retain only the "fck" clock.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> To: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

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

^ permalink raw reply

* Re: [PATCH v3] usb: dwc3: add disable u2mac linestate check quirk
From: Rob Herring @ 2017-04-28 13:41 UTC (permalink / raw)
  To: William Wu
  Cc: balbi-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	heiko-4mtYJXux2i+zQB+pC5nmwQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	frank.wang-TNX95d0MmH7DzftRWevZcw,
	huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
	briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA,
	daniel.meng-TNX95d0MmH7DzftRWevZcw,
	John.Youn-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <1492603898-25070-1-git-send-email-william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Wed, Apr 19, 2017 at 08:11:38PM +0800, William Wu wrote:
> This patch adds a quirk to disable USB 2.0 MAC linestate check
> during HS transmit. Refer the dwc3 databook, we can use it for
> some special platforms if the linestate not reflect the expected
> line state(J) during transmission.
> 
> When use this quirk, the controller implements a fixed 40-bit
> TxEndDelay after the packet is given on UTMI and ignores the
> linestate during the transmit of a token (during token-to-token
> and token-to-data IPGAP).
> 
> On some rockchip platforms (e.g. rk3399), it requires to disable
> the u2mac linestate check to decrease the SSPLIT token to SETUP
> token inter-packet delay from 566ns to 466ns, and fix the issue
> that FS/LS devices not recognized if inserted through USB 3.0 HUB.
> 
> Signed-off-by: William Wu <william.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> Changes in v3:
> - change quirk name
> - only read and write GUCTL1 if dwc3 version >= 2.50a
> 
> Changes in v2:
> - fix coding style
> 
>  Documentation/devicetree/bindings/usb/dwc3.txt |  2 ++

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 

>  drivers/usb/dwc3/core.c                        | 20 ++++++++++++++------
>  drivers/usb/dwc3/core.h                        |  4 ++++
>  3 files changed, 20 insertions(+), 6 deletions(-)
--
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/8] [RFC] dt-bindings: renesas: Document R-Car H3 and M3-W SiP DT bindings
From: Rob Herring @ 2017-04-28 13:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Mark Rutland, linux-renesas-soc, devicetree, linux-arm-kernel
In-Reply-To: <1492593351-13835-2-git-send-email-geert+renesas@glider.be>

On Wed, Apr 19, 2017 at 11:15:44AM +0200, Geert Uytterhoeven wrote:
> Document the SiP ("System-in-Package") versions of the R-Car H3 and M3-W
> SoCs, which contain an R-Car H3 or M3-W SoC, RAM, and HyperFlash.
> 
> Add their compatible values to all boards equipped with R-Car Gen3 SiPs.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Questions:
>   - Do we need more compatible values, for different configurations?
>     At least r8j7796 is available with either 2 GiB or 4 GiB of RAM,
>     possibly using RAM parts from different vendors.

Same die, just a different package? If so, I don't think you need a 
different compatible. It's going to be a different board from any 
non-SiP which should be enough to distinguish.

Rob

^ permalink raw reply

* Re: [PATCH 2/5 v2] clk: qcom: Elaborate on "active" clocks in the RPM clock bindings
From: Rob Herring @ 2017-04-28 13:35 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michael Turquette, Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170419091326.11226-2-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Wed, Apr 19, 2017 at 11:13:23AM +0200, Linus Walleij wrote:
> The concept of "active" clocks is just explained in a bried comment in the
> device driver, let's explain it a bit more in the device tree bindings
> so everyone understands this.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v1->v2:
> - Reword slighty in accordance with Stephens feedback.
> ---
>  Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

Despite my objections, you're just documenting what is already there.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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] dt-bindings: rockchip-dw-mshc: add optional rockchip,default-num-phases
From: Rob Herring @ 2017-04-28 13:34 UTC (permalink / raw)
  To: Shawn Lin
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Ziyuan Xu,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Doug Anderson, Jaehoon Chung,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1492592434-81312-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Wed, Apr 19, 2017 at 05:00:33PM +0800, Shawn Lin wrote:
> By default, dw_mmc-rockchip will execute tuning for each degree.
> So we won't miss every point of the good sample windows. However,
> probably the phases are linear inside the good sample window.
> Actually we don't need to do tuning for each degree so that we could
> save some time, for instance, probe the driver or resume from S3.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> index 520d61d..ea47ec0 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> @@ -31,6 +31,10 @@ Optional Properties:
>    probing, low speeds or in case where all phases work at tuning time.
>    If not specified 0 deg will be used.
>  
> +* rockchip,default-num-phases: The default number of times that the host
> +  execute tuning when needed. If not specified, the host will do tuning
> +  for 360 times, namely tuning for each degree.

How is it default when you specify it? I would think default here is 
360.

Should this be common?

Rob

^ permalink raw reply

* Re: [PATCH] drivers/of_iommu: ignore SMMU DT nodes with status 'disabled'
From: Ard Biesheuvel @ 2017-04-28 13:22 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	joro-zLv9SwRftAIdnm+yROfE0A, Robin Murphy,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland
In-Reply-To: <20170428131744.GL13675-5wv7dgnIgG8@public.gmane.org>

On 28 April 2017 at 14:17, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Fri, Apr 28, 2017 at 02:14:49PM +0100, Ard Biesheuvel wrote:
>> On 28 April 2017 at 14:11, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
>> > Hi Ard,
>> >
>> > [+ devicetree@]
>> >
>> > On Fri, Apr 14, 2017 at 01:43:15PM +0100, Ard Biesheuvel wrote:
>> >> DT nodes may have a status property, and if they do, such nodes should
>> >> only be considered present if the status property is set to 'okay'.
>> >>
>> >> Currently, we call the init function of IOMMUs described by the device
>> >> tree without taking this into account, which may result in the output
>> >> below on systems where some SMMUs may be legally disabled.
>> >>
>> >>  Failed to initialise IOMMU /smb/smmu@e0200000
>> >>  Failed to initialise IOMMU /smb/smmu@e0c00000
>> >>  arm-smmu e0a00000.smmu: probing hardware configuration...
>> >>  arm-smmu e0a00000.smmu: SMMUv1 with:
>> >>  arm-smmu e0a00000.smmu:  stage 2 translation
>> >>  arm-smmu e0a00000.smmu:  coherent table walk
>> >>  arm-smmu e0a00000.smmu:  stream matching with 32 register groups, mask 0x7fff
>> >>  arm-smmu e0a00000.smmu:  8 context banks (8 stage-2 only)
>> >>  arm-smmu e0a00000.smmu:  Supported page sizes: 0x60211000
>> >>  arm-smmu e0a00000.smmu:  Stage-2: 40-bit IPA -> 40-bit PA
>> >>  Failed to initialise IOMMU /smb/smmu@e0600000
>> >>  Failed to initialise IOMMU /smb/smmu@e0800000
>> >>
>> >> Since this is not an error condition, only call the init function if
>> >> the device is enabled, which also inhibits the spurious error messages.
>> >>
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> >> ---
>> >>  drivers/iommu/of_iommu.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> >> index 2683e9fc0dcf..2dd1206e6c0d 100644
>> >> --- a/drivers/iommu/of_iommu.c
>> >> +++ b/drivers/iommu/of_iommu.c
>> >> @@ -183,7 +183,7 @@ static int __init of_iommu_init(void)
>> >>       for_each_matching_node_and_match(np, matches, &match) {
>> >>               const of_iommu_init_fn init_fn = match->data;
>> >>
>> >> -             if (init_fn(np))
>> >> +             if (of_device_is_available(np) && init_fn(np))
>> >>                       pr_err("Failed to initialise IOMMU %s\n",
>> >>                               of_node_full_name(np));
>> >>       }
>> >
>> > Is there a definition of what status = "disabled" is supposed to mean for an
>> > IOMMU? For example, that could mean that the firmware has pre-programmed the
>> > SMMU with particular translations or memory attributes (a bit like the
>> > CCA=1, CPM=1, DACS=0 case in ACPI IORT), or even disabled DMA traffic
>> > altogether.
>> >
>> > So I think we'd need an update to the generic IOMMU binding text to say
>> > exactly what the semantics are supposed to be here.
>> >
>>
>> I agree that it might make sense to describe the behavior of the IOMMU
>> when it is left in the state we found it in. But that is not the same
>> as status=disabled.
>>
>> The DTS subtree contains loads and loads of boilerplate
>> configurations, where only some pieces are enabled in the final image
>> by setting status=okay. So a node that has status 'disabled' should be
>> treated as 'not present', not as 'present but can be ignored under
>> assumptions such and such'
>>
>> In other words, I think we are talking about two different issues here.
>
> I'm not so sure... if we have a master device that has an iommus= property
> pointing to an IOMMU with status="disabled", I really don't know whether we
> should:
>
>   1. Assume the master can do DMA with a 1:1 mapping of memory and no
>      changes to memory attributes
>
>   2. Assume the master can do DMA with a 1:1 mapping of memory, but
>      potentially with changes to the attributes
>
>   3. Assume the master can do DMA, but with some pre-existing translation
>      (what?)
>
>   4. Assume the master can't do DMA
>
> and I also don't know whether the "dma-coherent" property remains valid.
>

Ah yes. Good point.

So indeed, there should be some IOMMU specific status property that
can convey all of the above, or 1. and 4. at the minimum
--
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 01/10] pinctrl: generic: Add bi-directional and output-enable
From: Chris Brandt @ 2017-04-28 13:18 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
	Rob Herring, Mark Rutland, Russell King - ARM Linux,
	Linux-Renesas, linux-gpio@vger.kernel.org, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vcp_eMWrjZRX-x3=Tt3JRXcL_wWG43MymF=oyLLP4EJwg@mail.gmail.com>

On Friday, April 28, 2017, Andy Shevchenko wrote:
> > In the RZ/A1 HW manual you can kind of see that in 54.18 Port Control
> Logical Diagram (but that wasn't obvious to me at first).
> 
> Please, post a link to it or copy essential parts.


This board the RZ/A1 GENMAI board.
https://www.renesas.com/en-us/products/software-tools/boards-and-kits/evaluation-demo-solution-boards/genmai-cpu-board-rtk772100bc00000br.html


The schematic is included in the "User's manual"
https://www.renesas.com/en-us/doc/products/tool/doc/003/r20ut2596ej_r7s72100evum.pdf


The RZ/A1H Hardware manual is here:
https://www.renesas.com/en-us/document/hw-manual?hwLayerShowFlg=true&prdLayerId=186374&layerName=RZ%252FA1H&coronrService=document-prd-search&hwDocUrl=%2Fen-us%2Fdoc%2Fproducts%2Fmpumcu%2Fdoc%2Frz%2Fr01uh0403ej0300_rz_a1h.pdf&hashKey=54f335753742b5add524d4725b7242e6


Chapter 54 is the port/pin controller.

"54.18 Port Control Logical Diagram" is the diagram I was talking about. Note that is says "Note: This figure shows the logic for reference, not the circuit."

"54.3.13 Port Bidirection Control Register (PBDCn)" is the magic register needed to get some pins to work.


> I'm quite skeptical  that cheap hardware can implement something more
> costable than simplest open-source / open-drain + bias

I don't think this is an open-source / open-drain + bias issue. It's a "the internal signal paths are not getting hooked up correctly" issue.


Regardless, on this part, we needed a way to flag that some pins when put in some function modes needed 'an extra register setting'. At first we tried to sneak that info in with a simple #define in the pin/pinmux DT node properties. But, Linus didn't want it there so we had to make up a new generic property called "bi-directional".

What is your end goal here? Get "bi-directional" changed to something else?


Chris


^ permalink raw reply

* Re: [PATCH] drivers/of_iommu: ignore SMMU DT nodes with status 'disabled'
From: Will Deacon @ 2017-04-28 13:17 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	joro-zLv9SwRftAIdnm+yROfE0A, Robin Murphy,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland
In-Reply-To: <CAKv+Gu_G9fw0kwSetXmGjomc8Y_nu95O=rEVzgfafNRs0dtgvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Apr 28, 2017 at 02:14:49PM +0100, Ard Biesheuvel wrote:
> On 28 April 2017 at 14:11, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > Hi Ard,
> >
> > [+ devicetree@]
> >
> > On Fri, Apr 14, 2017 at 01:43:15PM +0100, Ard Biesheuvel wrote:
> >> DT nodes may have a status property, and if they do, such nodes should
> >> only be considered present if the status property is set to 'okay'.
> >>
> >> Currently, we call the init function of IOMMUs described by the device
> >> tree without taking this into account, which may result in the output
> >> below on systems where some SMMUs may be legally disabled.
> >>
> >>  Failed to initialise IOMMU /smb/smmu@e0200000
> >>  Failed to initialise IOMMU /smb/smmu@e0c00000
> >>  arm-smmu e0a00000.smmu: probing hardware configuration...
> >>  arm-smmu e0a00000.smmu: SMMUv1 with:
> >>  arm-smmu e0a00000.smmu:  stage 2 translation
> >>  arm-smmu e0a00000.smmu:  coherent table walk
> >>  arm-smmu e0a00000.smmu:  stream matching with 32 register groups, mask 0x7fff
> >>  arm-smmu e0a00000.smmu:  8 context banks (8 stage-2 only)
> >>  arm-smmu e0a00000.smmu:  Supported page sizes: 0x60211000
> >>  arm-smmu e0a00000.smmu:  Stage-2: 40-bit IPA -> 40-bit PA
> >>  Failed to initialise IOMMU /smb/smmu@e0600000
> >>  Failed to initialise IOMMU /smb/smmu@e0800000
> >>
> >> Since this is not an error condition, only call the init function if
> >> the device is enabled, which also inhibits the spurious error messages.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >>  drivers/iommu/of_iommu.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >> index 2683e9fc0dcf..2dd1206e6c0d 100644
> >> --- a/drivers/iommu/of_iommu.c
> >> +++ b/drivers/iommu/of_iommu.c
> >> @@ -183,7 +183,7 @@ static int __init of_iommu_init(void)
> >>       for_each_matching_node_and_match(np, matches, &match) {
> >>               const of_iommu_init_fn init_fn = match->data;
> >>
> >> -             if (init_fn(np))
> >> +             if (of_device_is_available(np) && init_fn(np))
> >>                       pr_err("Failed to initialise IOMMU %s\n",
> >>                               of_node_full_name(np));
> >>       }
> >
> > Is there a definition of what status = "disabled" is supposed to mean for an
> > IOMMU? For example, that could mean that the firmware has pre-programmed the
> > SMMU with particular translations or memory attributes (a bit like the
> > CCA=1, CPM=1, DACS=0 case in ACPI IORT), or even disabled DMA traffic
> > altogether.
> >
> > So I think we'd need an update to the generic IOMMU binding text to say
> > exactly what the semantics are supposed to be here.
> >
> 
> I agree that it might make sense to describe the behavior of the IOMMU
> when it is left in the state we found it in. But that is not the same
> as status=disabled.
> 
> The DTS subtree contains loads and loads of boilerplate
> configurations, where only some pieces are enabled in the final image
> by setting status=okay. So a node that has status 'disabled' should be
> treated as 'not present', not as 'present but can be ignored under
> assumptions such and such'
> 
> In other words, I think we are talking about two different issues here.

I'm not so sure... if we have a master device that has an iommus= property
pointing to an IOMMU with status="disabled", I really don't know whether we
should:

  1. Assume the master can do DMA with a 1:1 mapping of memory and no
     changes to memory attributes

  2. Assume the master can do DMA with a 1:1 mapping of memory, but
     potentially with changes to the attributes

  3. Assume the master can do DMA, but with some pre-existing translation
     (what?)

  4. Assume the master can't do DMA

and I also don't know whether the "dma-coherent" property remains valid.

Will
--
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] drivers/of_iommu: ignore SMMU DT nodes with status 'disabled'
From: Ard Biesheuvel @ 2017-04-28 13:14 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20170428131133.GJ13675-5wv7dgnIgG8@public.gmane.org>

On 28 April 2017 at 14:11, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Ard,
>
> [+ devicetree@]
>
> On Fri, Apr 14, 2017 at 01:43:15PM +0100, Ard Biesheuvel wrote:
>> DT nodes may have a status property, and if they do, such nodes should
>> only be considered present if the status property is set to 'okay'.
>>
>> Currently, we call the init function of IOMMUs described by the device
>> tree without taking this into account, which may result in the output
>> below on systems where some SMMUs may be legally disabled.
>>
>>  Failed to initialise IOMMU /smb/smmu@e0200000
>>  Failed to initialise IOMMU /smb/smmu@e0c00000
>>  arm-smmu e0a00000.smmu: probing hardware configuration...
>>  arm-smmu e0a00000.smmu: SMMUv1 with:
>>  arm-smmu e0a00000.smmu:  stage 2 translation
>>  arm-smmu e0a00000.smmu:  coherent table walk
>>  arm-smmu e0a00000.smmu:  stream matching with 32 register groups, mask 0x7fff
>>  arm-smmu e0a00000.smmu:  8 context banks (8 stage-2 only)
>>  arm-smmu e0a00000.smmu:  Supported page sizes: 0x60211000
>>  arm-smmu e0a00000.smmu:  Stage-2: 40-bit IPA -> 40-bit PA
>>  Failed to initialise IOMMU /smb/smmu@e0600000
>>  Failed to initialise IOMMU /smb/smmu@e0800000
>>
>> Since this is not an error condition, only call the init function if
>> the device is enabled, which also inhibits the spurious error messages.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/iommu/of_iommu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 2683e9fc0dcf..2dd1206e6c0d 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -183,7 +183,7 @@ static int __init of_iommu_init(void)
>>       for_each_matching_node_and_match(np, matches, &match) {
>>               const of_iommu_init_fn init_fn = match->data;
>>
>> -             if (init_fn(np))
>> +             if (of_device_is_available(np) && init_fn(np))
>>                       pr_err("Failed to initialise IOMMU %s\n",
>>                               of_node_full_name(np));
>>       }
>
> Is there a definition of what status = "disabled" is supposed to mean for an
> IOMMU? For example, that could mean that the firmware has pre-programmed the
> SMMU with particular translations or memory attributes (a bit like the
> CCA=1, CPM=1, DACS=0 case in ACPI IORT), or even disabled DMA traffic
> altogether.
>
> So I think we'd need an update to the generic IOMMU binding text to say
> exactly what the semantics are supposed to be here.
>

I agree that it might make sense to describe the behavior of the IOMMU
when it is left in the state we found it in. But that is not the same
as status=disabled.

The DTS subtree contains loads and loads of boilerplate
configurations, where only some pieces are enabled in the final image
by setting status=okay. So a node that has status 'disabled' should be
treated as 'not present', not as 'present but can be ignored under
assumptions such and such'

In other words, I think we are talking about two different issues here.

^ permalink raw reply


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