* Re: [PATCH v2 02/10] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Djtag dts bindings
From: Rob Herring @ 2016-12-19 16:31 UTC (permalink / raw)
To: Anurup M
Cc: mark.rutland, will.deacon, linux-kernel, devicetree,
linux-arm-kernel, anurup.m, zhangshaokun, tanxiaojun, xuwei5,
sanil.kumar, john.garry, gabriele.paoloni, shiju.jose,
wangkefeng.wang, linuxarm, shyju.pv
In-Reply-To: <1481129719-159487-1-git-send-email-anurup.m@huawei.com>
On Wed, Dec 07, 2016 at 11:55:19AM -0500, Anurup M wrote:
> From: Tan Xiaojun <tanxiaojun@huawei.com>
>
> Add Hisilicon HiP05/06/07 Djtag dts bindings for CPU and IO Die
>
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> ---
> .../devicetree/bindings/arm/hisilicon/djtag.txt | 41 ++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt b/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
> new file mode 100644
> index 0000000..733498e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
> @@ -0,0 +1,41 @@
> +The Hisilicon Djtag is an independent component which connects with some other
> +components in the SoC by Debug Bus. The djtag is available in CPU and IO dies
> +in the chip. The djtag controls access to connecting modules of CPU and IO
> +dies.
> +The various connecting components in CPU die (like L3 cache, L3 cache PMU etc.)
> +are accessed by djtag during real time debugging. In IO die there are connecting
> +components like RSA. These components appear as devices atatched to djtag bus.
> +
> +Hisilicon HiP05/06 djtag for CPU and HiP05 IO die
> +Required properties:
> + - compatible : "hisilicon,hisi-djtag-v1"
These need SoC specific compatible strings. They probably should
also include cpu or io in the compatible string. I would expect there
are things like events, triggers, or component connections for debug
that are SoC specifc.
> + - reg : Register address and size
> + - scl-id : The Super Cluster ID for CPU or IO die
> +
> +Hisilicon HiP06 djtag for IO die and HiP07 djtag for CPU and IO die
> +Required properties:
> + - compatible : "hisilicon,hisi-djtag-v2"
Same here.
> + - reg : Register address and size
> + - scl-id : The Super Cluster ID for CPU or IO die
> +
> +Example 1: Djtag for CPU die
> +
> + /* for Hisilicon HiP05 djtag for CPU Die */
> + djtag0: djtag@80010000 {
> + compatible = "hisilicon,hisi-djtag-v1";
> + reg = <0x0 0x80010000 0x0 0x10000>;
> + scl-id = <0x02>;
> +
> + /* All connecting components will appear as child nodes */
> + };
> +
> +Example 2: Djtag for IO die
> +
> + /* for Hisilicon HiP05 djtag for IO Die */
> + djtag1: djtag@d0000000 {
> + compatible = "hisilicon,hisi-djtag-v1";
> + reg = <0x0 0xd0000000 0x0 0x10000>;
> + scl-id = <0x01>;
> +
> + /* All connecting components will appear as child nodes */
> + };
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH v3 2/2] iio: adc: hx711: Add IIO driver for AVIA HX711
From: Lars-Peter Clausen @ 2016-12-19 16:28 UTC (permalink / raw)
To: Andreas Klinger, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg
In-Reply-To: <20161214161740.GA13896@andreas>
On 12/14/2016 05:17 PM, Andreas Klinger wrote:
[...]
> +#include <linux/err.h>
> +#include <linux/gpio.h>
Since you used the consumer API linux/gpio.h is not needed.
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define HX711_GAIN_32 2 /* gain = 32 for channel B */
> +#define HX711_GAIN_64 3 /* gain = 64 for channel A */
> +#define HX711_GAIN_128 1 /* gain = 128 for channel A */
> +
> +struct hx711_data {
> + struct device *dev;
> + struct gpio_desc *gpiod_sck;
> + struct gpio_desc *gpiod_dout;
> + int gain_pulse;
> + struct mutex lock;
> +};
> +
> +static int hx711_read(struct hx711_data *hx711_data)
> +{
> + int i, ret;
> + int value = 0;
> +
> + mutex_lock(&hx711_data->lock);
> +
> + if (hx711_reset(hx711_data)) {
If you reset the device before each conversion wont this clear the channel
and gain selection? Wouldn't the driver always read from channel A at 128
gain regardless of what has been selected?
> + dev_err(hx711_data->dev, "reset failed!");
> + mutex_unlock(&hx711_data->lock);
> + return -1;
If there is an error it should be propagated to the higher layers. At the
moment you only return a bogus conversion value.
> + }
> +
> + for (i = 0; i < 24; i++) {
> + value <<= 1;
> + ret = hx711_cycle(hx711_data);
> + if (ret)
> + value++;
> + }
> +
> + value ^= 0x800000;
> +
> + for (i = 0; i < hx711_data->gain_pulse; i++)
> + ret = hx711_cycle(hx711_data);
> +
> + mutex_unlock(&hx711_data->lock);
> +
> + return value;
> +}
> +
> +static int hx711_read_raw(struct iio_dev *iio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct hx711_data *hx711_data = iio_priv(iio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + *val = hx711_read(hx711_data);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
[...]
> +static struct attribute *hx711_attributes[] = {
> + &iio_dev_attr_gain.dev_attr.attr,
For IIO devices the gain is typically expressed through the scale attribute.
Which is kind of the inverse of gain. It would be good if this driver
follows this standard notation. The scale is the value of 1LSB in mV. So
this includes the resolution of the ADC, the reference voltage and any gain
that is applied to the input signal.
The possible values can be listed in the scale_available attribute.
> + NULL,
> +};
> +
> +static struct attribute_group hx711_attribute_group = {
> + .attrs = hx711_attributes,
> +};
> +
> +static const struct iio_info hx711_iio_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = hx711_read_raw,
> + .attrs = &hx711_attribute_group,
> +};
> +
> +static const struct iio_chan_spec hx711_chan_spec[] = {
> + { .type = IIO_VOLTAGE,
> + .info_mask_separate =
> + BIT(IIO_CHAN_INFO_RAW),
Given that there are two separate physical input channels this should be
expressed here and there should be two IIO channels for the device.
> + },
> +};
> +
> +static int hx711_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct hx711_data *hx711_data = NULL;
The = NULL is not needed as it is overwritten a few lines below.
> + struct iio_dev *iio;
> + int ret = 0;
Again = 0 no needed.
> +
> + iio = devm_iio_device_alloc(dev, sizeof(struct hx711_data));
> + if (!iio) {
> + dev_err(dev, "failed to allocate IIO device\n");
> + return -ENOMEM;
> + }
> +
> + hx711_data = iio_priv(iio);
> + hx711_data->dev = dev;
> +
> + mutex_init(&hx711_data->lock);
> +
> + hx711_data->gpiod_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_HIGH);
> + if (IS_ERR(hx711_data->gpiod_sck)) {
> + dev_err(dev, "failed to get sck-gpiod: err=%ld\n",
> + PTR_ERR(hx711_data->gpiod_sck));
> + return PTR_ERR(hx711_data->gpiod_sck);
> + }
> +
> + hx711_data->gpiod_dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_HIGH);
> + if (IS_ERR(hx711_data->gpiod_dout)) {
> + dev_err(dev, "failed to get dout-gpiod: err=%ld\n",
> + PTR_ERR(hx711_data->gpiod_dout));
> + return PTR_ERR(hx711_data->gpiod_dout);
> + }
> +
> + ret = gpiod_direction_input(hx711_data->gpiod_dout);
If dout is used as a input GPIO you should request it with GPIOD_IN. In that
case you can remove the gpiod_direction_input() call.
> + if (ret < 0) {
> + dev_err(hx711_data->dev, "gpiod_direction_input: %d\n", ret);
> + return ret;
> + }
> +
> + ret = gpiod_direction_output(hx711_data->gpiod_sck, 0);
Similar to above. If you want this to be a output GPIO with the default
value of 0 request it with GPIOD_OUT_LOW.
> + if (ret < 0) {
> + dev_err(hx711_data->dev, "gpiod_direction_output: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, iio);
There is no matching platform_get_drvdata() so this can probably be removed.
> +
> + iio->name = pdev->name;
This should be the part name. E.g. "hx711" in this case.
> + iio->dev.parent = &pdev->dev;
> + iio->info = &hx711_iio_info;
> + iio->modes = INDIO_DIRECT_MODE;
> + iio->channels = hx711_chan_spec;
> + iio->num_channels = ARRAY_SIZE(hx711_chan_spec);
> +
> + return devm_iio_device_register(dev, iio);
> +}
--
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] power: reset: add linkstation-reset driver
From: Roger Shimizu @ 2016-12-19 16:12 UTC (permalink / raw)
To: Andrew Lunn, Sebastian Reichel
Cc: Rob Herring, linux-pm, Ryan Tandy, Martin Michlmayr,
Sylver Bruneau, Herbert Valerio Riedel, Mark Rutland, devicetree
In-Reply-To: <20161219160345.GB16612@lunn.ch>
Thanks for your review!
On Tue, Dec 20, 2016 at 1:03 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > + reset {
>> > + compatible = "linkstation,power-off";
>> > + reg = <0x12100 0x100>;
>> > + clocks = <&core_clk 0>;
>> > + };
>>
>> This might be another user for UART slave device [0].
>> [0] https://lkml.org/lkml/2016/8/24/769
>>
>> Is the UART port used for anything else besides the reset
>> controller?
>
> I don't know much about these specific devices, but the qnap
> equivalent, there is a user space daemon which also talks to the
> microcontroller, for things like a temperature sensor, buzzer, etc.
>
> https://www.hellion.org.uk/qcontrol/
>
> So the UART can be in a messed up state, which is why the QNAP driver,
> which this code is modelled on, reset it back to a good state.
For Linkstation/KuroBox-Pro, it's quite similar, and the user-land program is
called micro-evtd [0], which is co-maintained by Ryan Tandy and me in Debian.
[0] https://tracker.debian.org/pkg/micro-evtd
>> [0] https://lkml.org/lkml/2016/8/24/769
Do I need to modify anything related to the above UART slave device?
Cheers,
--
Roger Shimizu, GMT +9 Tokyo
PGP/GPG: 4096R/6C6ACD6417B3ACB1
^ permalink raw reply
* Re: [PATCH v2 1/4] net: hix5hd2_gmac: add generic compatible string
From: Rob Herring @ 2016-12-19 16:04 UTC (permalink / raw)
To: Dongpo Li
Cc: David Miller, Mark Rutland, Michael Turquette, Stephen Boyd,
Russell King, Zhangfei Gao, Yisen Zhuang, salil.mehta,
Arnd Bergmann, Andrew Lunn, Jiancheng Xue, benjamin.chenhao,
caizhiyong, netdev, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <585796D6.1080407@hisilicon.com>
On Mon, Dec 19, 2016 at 2:14 AM, Dongpo Li <lidongpo@hisilicon.com> wrote:
> Hi Rob and David,
>
> On 2016/12/12 22:21, Rob Herring wrote:
>> On Mon, Dec 12, 2016 at 5:16 AM, Dongpo Li <lidongpo@hisilicon.com> wrote:
>>> Hi Rob,
>>>
>>> On 2016/12/10 6:35, Rob Herring wrote:
>>>> On Mon, Dec 05, 2016 at 09:27:58PM +0800, Dongpo Li wrote:
[...]
>>>>> @@ -20,7 +25,7 @@ Required properties:
>>>>>
>>>>> Example:
>>>>> gmac0: ethernet@f9840000 {
>>>>> - compatible = "hisilicon,hix5hd2-gmac";
>>>>> + compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>>>>
>>>> You can't just change compatible strings.
>>>>
>>> Okay, maybe I should name all the compatible string with the suffix "-gmac" instead of
>>> "-gemac". This can keep the compatible strings with the same suffix. Is this okay?
>>> Can I just add the generic compatible string without changing the SoCs compatible string?
>>> Like following:
>>> gmac0: ethernet@f9840000 {
>>> - compatible = "hisilicon,hix5hd2-gmac";
>>> + compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
>>
>> Yes, this is fine.
>
> As the patch series have been applied to net-next branch,
> in which way should I commit this compatible fix?
> Should I send a new patch fixing this compatible string error with "Fixes: xxx"?
> Looking forward to your reply!
Yes to both.
Rob
^ permalink raw reply
* Re: [PATCH v2] power: reset: add linkstation-reset driver
From: Andrew Lunn @ 2016-12-19 16:03 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Roger Shimizu, Rob Herring, linux-pm, Ryan Tandy,
Martin Michlmayr, Sylver Bruneau, Herbert Valerio Riedel,
Mark Rutland, devicetree
In-Reply-To: <20161219153802.vhcish35qyjbpevj@earth>
> > + reset {
> > + compatible = "linkstation,power-off";
> > + reg = <0x12100 0x100>;
> > + clocks = <&core_clk 0>;
> > + };
>
> This might be another user for UART slave device [0].
> [0] https://lkml.org/lkml/2016/8/24/769
>
> Is the UART port used for anything else besides the reset
> controller?
I don't know much about these specific devices, but the qnap
equivalent, there is a user space daemon which also talks to the
microcontroller, for things like a temperature sensor, buzzer, etc.
https://www.hellion.org.uk/qcontrol/
So the UART can be in a messed up state, which is why the QNAP driver,
which this code is modelled on, reset it back to a good state.
Andrew
^ permalink raw reply
* Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties
From: Rob Herring @ 2016-12-19 15:38 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, open list:MEDIA DRIVERS FOR RENESAS - FCP,
devicetree@vger.kernel.org, Tomi Valkeinen, Laurent Pinchart
In-Reply-To: <2900223.pppRAbRhe6@avalon>
On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote:
>> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
>> > On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
>> >> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
>> >>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
>> >>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>> >>>>> Document properties common to several display panels in a central
>> >>>>> location that can be referenced by the panel device tree bindings.
>> >>>>
>> >>>> Looks good. Just one comment...
>> >>>>
>> >>>> [...]
>> >>>>
>> >>>>> +Connectivity
>> >>>>> +------------
>> >>>>> +
>> >>>>> +- ports: Panels receive video data through one or multiple
>> >>>>> connections. While
>> >>>>> + the nature of those connections is specific to the panel type, the
>> >>>>> + connectivity is expressed in a standard fashion using ports as
>> >>>>> specified in
>> >>>>> + the device graph bindings defined in
>> >>>>> + Documentation/devicetree/bindings/graph.txt.
>> >>>>
>> >>>> We allow panels to either use graph binding or be a child of the
>> >>>> display controller.
>> >>>
>> >>> I knew that some display controllers use a phandle to the panel (see
>> >>> the fsl,panel and nvidia,panel properties), but I didn't know we had
>> >>> panels as children of display controller nodes. I don't think we should
>> >>> allow that for anything but DSI panels, as the DT hierarchy is based on
>> >>> control buses. Are you sure we have other panels instantiated through
>> >>> that mechanism ?
>> >
>> > Some panels have no control bus, so were do we place them?
>>
>> I'd say under the root node, like all similar control-less devices.
>>
>> > I would say the hierarchy is based on buses with a preference for the
>> > control bus when there are multiple buses. I'm not a fan of just sticking
>> > things are the top level.
>>
>> OK, so much for my comment a few lines up :-)
>>
>> The problem with placing non-DSI panels as children of the display
>> controller and not using OF graph is that the panel bindings become
>> dependent of the display controller being used. A display controller using
>> OF graph would require the panel to do the same, while a display controller
>> expecting a panel child node (with a specific name) would require DT
>> properties for the panel node.
Not sure I follow. They become dependent on the controller driver to
probe the panel, but the contents of the panel node would not be
controller dependent.
>> I'm also not sure the complexity of OF graph is really that prohibitive if
>> you compare it to panels as child nodes. To get the panel driver to bind to
>> the panel DT node the display controller driver would need to create a
>> platform device for the panel and register it. That's not very difficult,
>> but parsing a single port and endpoint isn't either (and we could even
>> provide a helper function for that, a version of of_drm_find_panel() that
>> would take as an argument the display controller device node instead of the
>> panel device node).
>
> Ping ?
>
> I'd like to standardize on one model for panel DT bindings, but I'm not sure
> that can be achieved given that we already have multiple competing models. In
> any case, is that blocking to merge this patch ? I only describe one
> connectivity model here as that's what my panel driver needs, but I have no
> issue adding more models later when needed. I believe this patch is a good
> step forward already.
It is an improvement which I appreciate, so yes I guess we can address
it later when needed.
Rob
^ permalink raw reply
* Re: [PATCH v2] power: reset: add linkstation-reset driver
From: Sebastian Reichel @ 2016-12-19 15:38 UTC (permalink / raw)
To: Roger Shimizu, Rob Herring
Cc: linux-pm, Andrew Lunn, Ryan Tandy, Martin Michlmayr,
Sylver Bruneau, Herbert Valerio Riedel, Mark Rutland, devicetree
In-Reply-To: <20161216100501.18173-1-rogershimizu@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 15717 bytes --]
Hi Roger,
On Fri, Dec 16, 2016 at 07:05:01PM +0900, Roger Shimizu wrote:
> Buffalo Linkstation / KuroBox and their variants need magic command
> sending to UART1 to power-off.
>
> Power driver linkstation-reset implements the magic command and I/O
> routine, which come from files listed below:
> - arch/arm/mach-orion5x/kurobox_pro-setup.c
> - arch/arm/mach-orion5x/terastation_pro2-setup.c
Ok.
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Martin Michlmayr <tbm@cyrius.com>
> Cc: Sylver Bruneau <sylver.bruneau@googlemail.com>
> Cc: Herbert Valerio Riedel <hvr@gnu.org>
> Reported-by: Ryan Tandy <ryan@nardis.ca>
> Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
> ---
> Dear Sebastian,
>
> Kurobox-Pro (and variants) need more commands sending to UART1 to shutdown.
> So here I make this patch series to let current qnap-poweroff implementation
> be able to handle such case.
>
> I already tested this change on Kurobox-Pro and Linkstation LS-GL devices,
> with a modified device-tree file. (Previous device-tree of kurobox-pro invokes
> restart-poweroff, so it simply restarts.)
>
> Thank you and look forward to your feedback!
>
> Dear Andrew,
>
> Thanks for your 2nd review!
>
> So I accept your suggestion and make the new driver for linkstation series.
>
> Changes:
> v0 => v1:
> - Update 0003 to split kuroboxpro related code into kuroboxpro-common.c
> v1 => v2:
> - Slipt off linkstation/kuroboxpro related code to linkstation-reset.c
> Because linkstation before kuroboxpro also need this driver to power
> off properly. It's more proper to call it linkstation driver.
>
> Cheers,
> --
> Roger Shimizu, GMT +9 Tokyo
> PGP/GPG: 4096R/6C6ACD6417B3ACB1
>
> .../bindings/power/reset/linkstation-reset.txt | 26 ++++
> drivers/power/reset/Kconfig | 10 ++
> drivers/power/reset/Makefile | 1 +
> drivers/power/reset/linkstation-common.c | 124 +++++++++++++++
> drivers/power/reset/linkstation-common.h | 8 +
> drivers/power/reset/linkstation-reset.c | 172 +++++++++++++++++++++
> 6 files changed, 341 insertions(+)
With this being its own driver please merge linkstation-common and
linkstation-reset. The common part is only used by linkstation-reset
anyways.
> create mode 100644 Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
This patch is missing Cc for DT binding people (check "OPEN FIRMWARE
AND FLATTENED DEVICE TREE BINDINGS" in MAINTAINERS file).
> create mode 100644 drivers/power/reset/linkstation-common.c
> create mode 100644 drivers/power/reset/linkstation-common.h
> create mode 100644 drivers/power/reset/linkstation-reset.c
>
> diff --git a/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> new file mode 100644
> index 0000000..815e340
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/linkstation-reset.txt
> @@ -0,0 +1,26 @@
> +* Buffalo Linkstation Reset Driver
> +
> +Power of some Buffalo Linkstation or KuroBox Pro is managed by
> +micro-controller, which connects to UART1. After being fed from UART1
> +by a few magic numbers, the so-called power-off command,
> +the micro-controller will turn power off the device.
> +
> +This is very similar to QNAP or Synology NAS devices, which is
> +described in qnap-poweroff.txt, however the command is much simpler,
> +only 1-byte long and without checksums.
> +
> +This driver adds a handler to pm_power_off which is called to turn the
> +power off.
> +
> +Required Properties:
> +- compatible: Should be "linkstation,power-off"
> +- reg: Address and length of the register set for UART1
> +- clocks: tclk clock
> +
> +Example:
> +
> + reset {
> + compatible = "linkstation,power-off";
> + reg = <0x12100 0x100>;
> + clocks = <&core_clk 0>;
> + };
This might be another user for UART slave device [0].
[0] https://lkml.org/lkml/2016/8/24/769
Is the UART port used for anything else besides the reset
controller?
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index c74c3f6..77c44ca 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -98,6 +98,16 @@ config POWER_RESET_IMX
> say N here or disable in dts to make sure pm_power_off never be
> overwrote wrongly by this driver.
>
> +config POWER_RESET_LINKSTATION
> + bool "Buffalo Linkstation and its variants reset driver"
> + depends on OF_GPIO && PLAT_ORION
> + help
> + This driver supports power off Buffalo Linkstation / KuroBox Pro
> + NAS and their variants by sending commands to the micro-controller
> + which controls the main power.
> +
> + Say Y if you have a Buffalo Linkstation / KuroBox Pro NAS.
> +
> config POWER_RESET_MSM
> bool "Qualcomm MSM power-off driver"
> depends on ARCH_QCOM
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index 1be307c..520afbe 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
> obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o
> obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
> obj-$(CONFIG_POWER_RESET_IMX) += imx-snvs-poweroff.o
> +obj-$(CONFIG_POWER_RESET_LINKSTATION) += linkstation-reset.o linkstation-common.o
> obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
> obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
> obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
> diff --git a/drivers/power/reset/linkstation-common.c b/drivers/power/reset/linkstation-common.c
> new file mode 100644
> index 0000000..a6d0930
> --- /dev/null
> +++ b/drivers/power/reset/linkstation-common.c
> @@ -0,0 +1,124 @@
> +/*
> + * Common I/O routine for micro-controller of Buffalo Linkstation
> + * and its variants.
> + *
> + * Copyright (C) 2016 Roger Shimizu <rogershimizu@gmail.com>
> + *
> + * Based on the code from:
> + *
> + * Copyright (C) 2008 Sylver Bruneau <sylver.bruneau@googlemail.com>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@gnu.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/serial_reg.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include "linkstation-common.h"
> +
> +static int uart1_micon_read(void *base, unsigned char *buf, int count)
> +{
> + int i;
> + int timeout;
> +
> + for (i = 0; i < count; i++) {
> + timeout = 10;
> +
> + while (!(readl(UART1_REG(LSR)) & UART_LSR_DR)) {
> + if (--timeout == 0)
> + break;
> + udelay(1000);
> + }
> +
> + if (timeout == 0)
> + break;
> + buf[i] = readl(UART1_REG(RX));
> + }
> +
> + /* return read bytes */
> + return i;
> +}
> +
> +static int uart1_micon_write(void *base, const unsigned char *buf, int count)
> +{
> + int i = 0;
> +
> + while (count--) {
> + while (!(readl(UART1_REG(LSR)) & UART_LSR_THRE))
> + barrier();
> + writel(buf[i++], UART1_REG(TX));
> + }
> +
> + return 0;
> +}
> +
> +int uart1_micon_send(void *base, const unsigned char *data, int count)
> +{
> + int i;
> + unsigned char checksum = 0;
> + unsigned char recv_buf[40];
> + unsigned char send_buf[40];
> + unsigned char correct_ack[3];
> + int retry = 2;
> +
> + /* Generate checksum */
> + for (i = 0; i < count; i++)
> + checksum -= data[i];
> +
> + do {
> + /* Send data */
> + uart1_micon_write(base, data, count);
> +
> + /* send checksum */
> + uart1_micon_write(base, &checksum, 1);
> +
> + if (uart1_micon_read(base, recv_buf, sizeof(recv_buf)) <= 3) {
> + printk(KERN_ERR ">%s: receive failed.\n", __func__);
> +
> + /* send preamble to clear the receive buffer */
> + memset(&send_buf, 0xff, sizeof(send_buf));
> + uart1_micon_write(base, send_buf, sizeof(send_buf));
> +
> + /* make dummy reads */
> + mdelay(100);
> + uart1_micon_read(base, recv_buf, sizeof(recv_buf));
> + } else {
> + /* Generate expected ack */
> + correct_ack[0] = 0x01;
> + correct_ack[1] = data[1];
> + correct_ack[2] = 0x00;
> +
> + /* checksum Check */
> + if ((recv_buf[0] + recv_buf[1] + recv_buf[2] +
> + recv_buf[3]) & 0xFF) {
> + printk(KERN_ERR ">%s: Checksum Error : "
> + "Received data[%02x, %02x, %02x, %02x]"
> + "\n", __func__, recv_buf[0],
> + recv_buf[1], recv_buf[2], recv_buf[3]);
> + } else {
> + /* Check Received Data */
> + if (correct_ack[0] == recv_buf[0] &&
> + correct_ack[1] == recv_buf[1] &&
> + correct_ack[2] == recv_buf[2]) {
> + /* Interval for next command */
> + mdelay(10);
> +
> + /* Receive ACK */
> + return 0;
> + }
> + }
> + /* Received NAK or illegal Data */
> + printk(KERN_ERR ">%s: Error : NAK or Illegal Data "
> + "Received\n", __func__);
> + }
> + } while (retry--);
> +
> + /* Interval for next command */
> + mdelay(10);
> +
> + return -1;
> +}
> diff --git a/drivers/power/reset/linkstation-common.h b/drivers/power/reset/linkstation-common.h
> new file mode 100644
> index 0000000..89c64a9
> --- /dev/null
> +++ b/drivers/power/reset/linkstation-common.h
> @@ -0,0 +1,8 @@
> +#ifndef __LINKSTATION_COMMON_H__
> +#define __LINKSTATION_COMMON_H__
> +
> +#define UART1_REG(x) (base + ((UART_##x) << 2))
> +
> +int uart1_micon_send(void *base, const unsigned char *data, int count);
> +
> +#endif
> diff --git a/drivers/power/reset/linkstation-reset.c b/drivers/power/reset/linkstation-reset.c
> new file mode 100644
> index 0000000..78a0137
> --- /dev/null
> +++ b/drivers/power/reset/linkstation-reset.c
> @@ -0,0 +1,172 @@
> +/*
> + * Buffalo Linkstation power reset driver.
> + * It may also be used on following devices:
> + * - Buffalo Linkstation HG
> + * - KuroBox HG
> + * - Buffalo KURO-NAS/T4
> + * - KuroBox Pro
> + * - Buffalo Linkstation Pro (LS-GL)
> + * - Buffalo Terastation Pro II/Live
> + * - Buffalo Linkstation Duo (LS-WTGL)
> + * - Buffalo Linkstation Mini (LS-WSGL)
> + *
> + * Copyright (C) 2016 Roger Shimizu <rogershimizu@gmail.com>
> + *
> + * Based on the code from:
> + *
> + * Copyright (C) 2012 Andrew Lunn <andrew@lunn.ch>
> + * Copyright (C) 2009 Martin Michlmayr <tbm@cyrius.com>
> + * Copyright (C) 2008 Byron Bradley <byron.bbradley@gmail.com>
> + *
> + * 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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/serial_reg.h>
> +#include <linux/kallsyms.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include "linkstation-common.h"
> +
> +#define MICON_CMD_SIZE 4
> +
> +/* 4-byte magic hello command to UART1-attached microcontroller */
> +static const unsigned char linkstation_micon_magic[] = {
> + 0x1b,
> + 0x00,
> + 0x07,
> + 0x00
> +};
4-byte magic hello command? Those are used as uart configuration as
far as I can see. Just move this directly into reset_cfg:
struct reset_cfg {
u32 baud;
u8 lcr;
u8 ier;
u8 fcr;
u8 mcr;
const unsigned char (*cmd)[MICON_CMD_SIZE];
};
> +// for each row, first byte is the size of command
> +static const unsigned char linkstation_power_off_cmd[][MICON_CMD_SIZE] = {
> + { 3, 0x01, 0x35, 0x00},
> + { 2, 0x00, 0x0c},
> + { 2, 0x00, 0x06},
> + {}
> +};
> +
> +struct reset_cfg {
> + u32 baud;
> + const unsigned char *magic;
> + const unsigned char (*cmd)[MICON_CMD_SIZE];
> +};
> +
> +static const struct reset_cfg linkstation_power_off_cfg = {
> + .baud = 38400,
> + .magic = linkstation_micon_magic,
> + .cmd = linkstation_power_off_cmd,
> +};
> +
> +static const struct of_device_id linkstation_reset_of_match_table[] = {
> + { .compatible = "linkstation,power-off",
> + .data = &linkstation_power_off_cfg,
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, linkstation_reset_of_match_table);
> +
> +static void __iomem *base;
> +static unsigned long tclk;
> +static const struct reset_cfg *cfg;
> +
> +static void linkstation_reset(void)
> +{
> + const unsigned divisor = ((tclk + (8 * cfg->baud)) / (16 * cfg->baud));
> +
> + pr_err("%s: triggering power-off...\n", __func__);
> +
> + /* hijack UART1 and reset into sane state */
> + writel(0x83, UART1_REG(LCR));
> + writel(divisor & 0xff, UART1_REG(DLL));
> + writel((divisor >> 8) & 0xff, UART1_REG(DLM));
> + writel(cfg->magic[0], UART1_REG(LCR));
> + writel(cfg->magic[1], UART1_REG(IER));
> + writel(cfg->magic[2], UART1_REG(FCR));
> + writel(cfg->magic[3], UART1_REG(MCR));
> +
> + /* send the power-off command to PIC */
> + if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
> + /* if it's simply one-byte command, send it directly */
> + writel(cfg->cmd[0][1], UART1_REG(TX));
> + }
I guess this optimization can be dropped and you can directly
call the for loop with uart1_micon_send().
> + else {
> + int i;
> + for(i = 0; cfg->cmd[i][0] > 0; i ++) {
> + /* [0] is size of the command; command starts from [1] */
> + uart1_micon_send(base, &(cfg->cmd[i][1]), cfg->cmd[i][0]);
> + }
> + }
> +}
> +
> +static int linkstation_reset_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct resource *res;
> + struct clk *clk;
> + char symname[KSYM_NAME_LEN];
> +
> + const struct of_device_id *match =
> + of_match_node(linkstation_reset_of_match_table, np);
> + cfg = match->data;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "Missing resource");
> + return -EINVAL;
> + }
> +
> + base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!base) {
> + dev_err(&pdev->dev, "Unable to map resource");
> + return -EINVAL;
> + }
> +
> + /* We need to know tclk in order to calculate the UART divisor */
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Clk missing");
> + return PTR_ERR(clk);
> + }
> +
> + tclk = clk_get_rate(clk);
> +
> + /* Check that nothing else has already setup a handler */
> + if (pm_power_off) {
> + lookup_symbol_name((ulong)pm_power_off, symname);
> + dev_err(&pdev->dev,
> + "pm_power_off already claimed %p %s",
> + pm_power_off, symname);
> + return -EBUSY;
> + }
> + pm_power_off = linkstation_reset;
> +
> + return 0;
> +}
> +
> +static int linkstation_reset_remove(struct platform_device *pdev)
> +{
> + pm_power_off = NULL;
> + return 0;
> +}
> +
> +static struct platform_driver linkstation_reset_driver = {
> + .probe = linkstation_reset_probe,
> + .remove = linkstation_reset_remove,
> + .driver = {
> + .name = "linkstation_reset",
> + .of_match_table = of_match_ptr(linkstation_reset_of_match_table),
> + },
> +};
> +
> +module_platform_driver(linkstation_reset_driver);
> +
> +MODULE_AUTHOR("Roger Shimizu <rogershimizu@gmail.com>");
> +MODULE_DESCRIPTION("KuroBox Pro Reset driver");
> +MODULE_LICENSE("GPL v2");
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v4 2/4] dt-bindings: net: add EEE capability constants
From: Jerome Brunet @ 2016-12-19 15:16 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Florian Fainelli, Alexandre TORGUE, Andrew Lunn,
Martin Blumenstingl, netdev, Neil Armstrong, linux-kernel,
Yegor Yefremov, Julia Lawall, Andre Roth, Kevin Hilman,
Carlo Caione, Giuseppe Cavallaro, linux-amlogic,
Andreas Färber, linux-arm-kernel
In-Reply-To: <20161205143942.f3w6nmp3jvmrk5es@rob-hp-laptop>
Hi Rob,
First, Thx for this information and sorry for this late reply
As you may have seen yourself, there was little bit of confusion while
discussing this patch series.
The point is the v3 was applied before your reply (patches 2 and 3 not
combined unfortunately).
Because of this confusion, the series needed a few fixes witch removes
the previously added bindings [0].
This time, I made sure to modify (remove) the bindings along with the
documentation.
[0]: http://lkml.kernel.org/r/1482159938-13239-1-git-send-email-jbrunet
@baylibre.com
Cheers
Jerome
On Mon, 2016-12-05 at 08:39 -0600, Rob Herring wrote:
> On Mon, Nov 28, 2016 at 04:50:26PM +0100, Jerome Brunet wrote:
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> > Tested-by: Andreas Färber <afaerber@suse.de>
> > Tested-by: Neil Armstrong <narmstrong@baylibre.com>
> > ---
> > include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> > create mode 100644 include/dt-bindings/net/mdio.h
>
> Seems changes are wanted on this, but patches 2 and 3 should be
> combined. The header is part of the binding doc.
>
> Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Volodymyr Bendiuga @ 2016-12-19 15:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: Volodymyr Bendiuga, Romain Perier, Vivien Didelot,
Florian Fainelli, Jason Cooper, Sebastian Hesselbarth,
Gregory Clement, netdev, devicetree, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala, linux-arm-kernel,
Thomas Petazzoni, Nadav Haklai
In-Reply-To: <20161219150759.GG10048@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 506 bytes --]
Hi,
Sure, will do that.
Regards,
Volodymyr
On Mon, Dec 19, 2016 at 4:07 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Dec 19, 2016 at 04:04:32PM +0100, Volodymyr Bendiuga wrote:
> > Hi Andrew,
> >
> > No, it did not get accepted. Or at least I did not see
> > David accepting it. Let me know if I should resubmit it.
>
> Hi Volodymyr
>
> Please do resend it. Probably netdev will reopen sometime after the
> 25th.
>
> Don't forget to include the reviewed-by i gave.
>
> Thanks
>
> Andrew
>
[-- Attachment #2: Type: text/html, Size: 975 bytes --]
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: sun8i: add opp-v2 table for A33
From: Icenowy Zheng @ 2016-12-19 15:10 UTC (permalink / raw)
To: Quentin Schulz
Cc: linux-arm-kernel, linux-sunxi, linux-kernel, Hans de Goede,
devicetree, Maxime Ripard, linux-clk, Chen-Yu Tsai
2016年12月19日 22:30于 Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>写道：
>
> On 19/12/2016 15:06, Icenowy Zheng wrote:
> >
> >
> > 19.12.2016, 16:54, "Chen-Yu Tsai" <wens-jdAy2FN1RRM@public.gmane.org>:
> >> On Mon, Dec 19, 2016 at 4:46 PM, Maxime Ripard
> >> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >>> On Fri, Dec 16, 2016 at 02:27:54AM +0800, Icenowy Zheng wrote:
> >>>> An operating point table is needed for the cpu frequency adjusting to
> >>>> work.
> >>>>
> >>>> The operating point table is converted from the common value in
> >>>> extracted script.fex from many A33 board/tablets.
> >>>>
> >>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> >>>> ---
> >>>> Changes since v1:
> >>>> - Fix format problem (blank lines).
> >>>> - Removed the 1.344GHz operating point, as it's overvoltage and overclocked.
> >>>>
> >>>> This patch depends on the following patchset:
> >>>>
> >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/473962.html
> >>>>
> >>>> It's the v2 of the [PATCH 4/6] in this patchset.
> >>>>
> >>>> I think this operating point table may also apply to A23, as there's no
> >>>> difference except the points over 1.2GHz between A23 and A33's stock dvfs table.
> >>>>
> >>>> But as A23 CCU may not have the necessary fixes, I won't add the table to A23
> >>>> now.
> >>>>
> >>>> Chen-Yu, could you test the CCU fixes I described in the patchset above on A23,
> >>>> then test this operating points table?
> >>>>
> >>>> If it's necessary, you can send out the CCU fixes and add one more patch that
> >>>> moves this opp-v2 table to sun8i-a23-a33.dtsi .
> >>>>
> >>>> arch/arm/boot/dts/sun8i-a33.dtsi | 35 +++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 35 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> >>>> index 504996cbee29..0f5b2af72981 100644
> >>>> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> >>>> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> >>>> @@ -46,7 +46,42 @@
> >>>> #include <dt-bindings/dma/sun4i-a10.h>
> >>>>
> >>>> / {
> >>>> + cpu0_opp_table: opp_table0 {
> >>>> + compatible = "operating-points-v2";
> >>>> + opp-shared;
> >>>> +
> >>>> + opp@648000000 {
> >>>> + opp-hz = /bits/ 64 <648000000>;
> >>>> + opp-microvolt = <1040000>;
> >>>> + clock-latency-ns = <244144>; /* 8 32k periods */
> >>>> + };
> >>>> +
> >>>> + opp@816000000 {
> >>>> + opp-hz = /bits/ 64 <816000000>;
> >>>> + opp-microvolt = <1100000>;
> >>>> + clock-latency-ns = <244144>; /* 8 32k periods */
> >>>> + };
> >>>> +
> >>>> + opp@1008000000 {
> >>>> + opp-hz = /bits/ 64 <1008000000>;
> >>>> + opp-microvolt = <1200000>;
> >>>> + clock-latency-ns = <244144>; /* 8 32k periods */
> >>>> + };
> >>>> +
> >>>> + opp@1200000000 {
> >>>> + opp-hz = /bits/ 64 <1200000000>;
> >>>> + opp-microvolt = <1320000>;
> >>>> + clock-latency-ns = <244144>; /* 8 32k periods */
> >>>> + };
> >>>> + };
> >>>> +
>
> Also, there are a lot more operating points for the A33, see:
> https://github.com/QSchulz/linux/blob/v4.9-rc4_adc_a31_v7/cpufreq_a33/arch/arm/boot/dts/sun8i-a33.dtsi#L323-L340
>
> They are present in the Allwinner Linux source code and in the fex of
> all A33-based boards.
>
> Is there a reason for not adding all opp?
I just didn't see them...
Will add them in a further patch.
>
> Quentin
>
> --
> Quentin Schulz, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Andrew Lunn @ 2016-12-19 15:07 UTC (permalink / raw)
To: Volodymyr Bendiuga
Cc: Romain Perier, Vivien Didelot, Florian Fainelli, Jason Cooper,
Sebastian Hesselbarth, Gregory Clement, netdev, devicetree,
Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel, Thomas Petazzoni, Nadav Haklai
In-Reply-To: <CABHmqqDqhJa6N=Ayu-GFNyp4g90dXzTje6o8qfCEP2DacCFWdQ@mail.gmail.com>
On Mon, Dec 19, 2016 at 04:04:32PM +0100, Volodymyr Bendiuga wrote:
> Hi Andrew,
>
> No, it did not get accepted. Or at least I did not see
> David accepting it. Let me know if I should resubmit it.
Hi Volodymyr
Please do resend it. Probably netdev will reopen sometime after the
25th.
Don't forget to include the reviewed-by i gave.
Thanks
Andrew
^ permalink raw reply
* [PATCH net 3/3] dt: bindings: net: use boolean dt properties for eee broken modes
From: Jerome Brunet @ 2016-12-19 15:05 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Martin Blumenstingl,
Andre Roth, Andrew Lunn, Neil Armstrong, linux-amlogic,
linux-arm-kernel, linux-kernel, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1482159938-13239-1-git-send-email-jbrunet@baylibre.com>
The patches regarding eee-broken-modes was merged before all people
involved could find an agreement on the best way to move forward.
While we agreed on having a DT property to mark particular modes as broken,
the value used for eee-broken-modes mapped the phy register in very direct
way. Because of this, the concern is that it could be used to implement
configuration policies instead of describing a broken HW.
In the end, having a boolean property for each mode seems to be preferred
over one bit field value mapping the register (too) directly.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++--
include/dt-bindings/net/mdio.h | 19 -------------------
2 files changed, 8 insertions(+), 21 deletions(-)
delete mode 100644 include/dt-bindings/net/mdio.h
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 54749b60a466..ff1bc4b1bb3b 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -38,8 +38,14 @@ Optional Properties:
- enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to
compensate for the board being designed with the lanes swapped.
-- eee-broken-modes: Bits to clear in the MDIO_AN_EEE_ADV register to
- disable EEE broken modes.
+- eee-broken-100tx:
+- eee-broken-1000t:
+- eee-broken-10gt:
+- eee-broken-1000kx:
+- eee-broken-10gkx4:
+- eee-broken-10gkr:
+ Mark the corresponding energy efficient ethernet mode as broken and
+ request the ethernet to stop advertising it.
Example:
diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
deleted file mode 100644
index 99c6d903d439..000000000000
--- a/include/dt-bindings/net/mdio.h
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * This header provides generic constants for ethernet MDIO bindings
- */
-
-#ifndef _DT_BINDINGS_NET_MDIO_H
-#define _DT_BINDINGS_NET_MDIO_H
-
-/*
- * EEE capability Advertisement
- */
-
-#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
-#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
-#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
-#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
-#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
-#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
-
-#endif
--
2.7.4
^ permalink raw reply related
* [PATCH net 2/3] net: phy: use boolean dt properties for eee broken modes
From: Jerome Brunet @ 2016-12-19 15:05 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Martin Blumenstingl,
Andre Roth, Andrew Lunn, Neil Armstrong, linux-amlogic,
linux-arm-kernel, linux-kernel, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1482159938-13239-1-git-send-email-jbrunet@baylibre.com>
The patches regarding eee-broken-modes was merged before all people
involved could find an agreement on the best way to move forward.
While we agreed on having a DT property to mark particular modes as broken,
the value used for eee-broken-modes mapped the phy register in very direct
way. Because of this, the concern is that it could be used to implement
configuration policies instead of describing a broken HW.
In the end, having a boolean property for each mode seems to be preferred
over one bit field value mapping the register (too) directly.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/phy_device.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ee5ebadb1463..92b08383cafa 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1665,7 +1665,7 @@ static void of_set_phy_supported(struct phy_device *phydev)
static void of_set_phy_eee_broken(struct phy_device *phydev)
{
struct device_node *node = phydev->mdio.dev.of_node;
- u32 broken;
+ u32 broken = 0;
if (!IS_ENABLED(CONFIG_OF_MDIO))
return;
@@ -1673,8 +1673,20 @@ static void of_set_phy_eee_broken(struct phy_device *phydev)
if (!node)
return;
- if (!of_property_read_u32(node, "eee-broken-modes", &broken))
- phydev->eee_broken_modes = broken;
+ if (of_property_read_bool(node, "eee-broken-100tx"))
+ broken |= MDIO_EEE_100TX;
+ if (of_property_read_bool(node, "eee-broken-1000t"))
+ broken |= MDIO_EEE_1000T;
+ if (of_property_read_bool(node, "eee-broken-10gt"))
+ broken |= MDIO_EEE_10GT;
+ if (of_property_read_bool(node, "eee-broken-1000kx"))
+ broken |= MDIO_EEE_1000KX;
+ if (of_property_read_bool(node, "eee-broken-10gkx4"))
+ broken |= MDIO_EEE_10GKX4;
+ if (of_property_read_bool(node, "eee-broken-10gkr"))
+ broken |= MDIO_EEE_10GKR;
+
+ phydev->eee_broken_modes = broken;
}
/**
--
2.7.4
^ permalink raw reply related
* [PATCH net 1/3] net: phy: fix sign type error in genphy_config_eee_advert
From: Jerome Brunet @ 2016-12-19 15:05 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Jerome Brunet, Carlo Caione, Kevin Hilman, Martin Blumenstingl,
Andre Roth, Andrew Lunn, Neil Armstrong, linux-amlogic,
linux-arm-kernel, linux-kernel, Julia Lawall, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1482159938-13239-1-git-send-email-jbrunet@baylibre.com>
In genphy_config_eee_advert, the return value of phy_read_mmd_indirect is
checked to know if the register could be accessed but the result is
assigned to a 'u32'.
Changing to 'int' to correctly get errors from phy_read_mmd_indirect.
Fixes: d853d145ea3e ("net: phy: add an option to disable EEE advertisement")
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/phy_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9c06f8028f0c..ee5ebadb1463 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1187,8 +1187,8 @@ static int genphy_config_advert(struct phy_device *phydev)
*/
static int genphy_config_eee_advert(struct phy_device *phydev)
{
- u32 broken = phydev->eee_broken_modes;
- u32 old_adv, adv;
+ int broken = phydev->eee_broken_modes;
+ int old_adv, adv;
/* Nothing to disable */
if (!broken)
--
2.7.4
^ permalink raw reply related
* [PATCH net 0/3] Fix integration of eee-broken-modes
From: Jerome Brunet @ 2016-12-19 15:05 UTC (permalink / raw)
To: netdev, devicetree, Florian Fainelli
Cc: Andrew Lunn, Neil Armstrong, Martin Blumenstingl, Kevin Hilman,
linux-kernel, Yegor Yefremov, Julia Lawall, Andre Roth,
Carlo Caione, linux-amlogic, Andreas Färber,
linux-arm-kernel, Jerome Brunet
The purpose of this series is to fix the integration of the ethernet phy
property "eee-broken-modes" [0]
The v3 of this series has been merged, missing a fix (error reported by
kbuild robot) available in the v4 [1]
More importantly, Florian opposed adding a DT property mapping a device
register this directly [2]. The concern was that the property could be
abused to implement platform configuration policy. After discussing it,
I think we agreed that such information about the HW (defect) should appear
in the platform DT. However, the preferred way is to add a boolean property
for each EEE broken mode.
[0]: http://lkml.kernel.org/r/1480326409-25419-1-git-send-email-jbrunet@baylibre.com
[1]: http://lkml.kernel.org/r/1480348229-25672-1-git-send-email-jbrunet@baylibre.com
[2]: http://lkml.kernel.org/r/e14a3b0c-dc34-be14-48b3-518a0ad0c080@gmail.com
Jerome Brunet (3):
net: phy: fix sign type error in genphy_config_eee_advert
net: phy: use boolean dt properties for eee broken modes
dt: bindings: net: use boolean dt properties for eee broken modes
Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++--
drivers/net/phy/phy_device.c | 22 +++++++++++++++++-----
include/dt-bindings/net/mdio.h | 19 -------------------
3 files changed, 25 insertions(+), 26 deletions(-)
delete mode 100644 include/dt-bindings/net/mdio.h
--
2.7.4
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Volodymyr Bendiuga @ 2016-12-19 15:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: Romain Perier, Vivien Didelot, Florian Fainelli, Jason Cooper,
Sebastian Hesselbarth, Gregory Clement, netdev, devicetree,
Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel, Thomas Petazzoni, Nadav Haklai
In-Reply-To: <20161219150008.GE10048@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]
Hi Andrew,
No, it did not get accepted. Or at least I did not see
David accepting it. Let me know if I should resubmit it.
Regards,
Volodymyr
On Mon, Dec 19, 2016 at 4:00 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Dec 19, 2016 at 03:56:34PM +0100, Romain Perier wrote:
> > Hi Andrew,
> >
> > Le 19/12/2016 à 15:38, Andrew Lunn a écrit :
> > >On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
> > >>Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
> > >>zero of sw_addr is 0x1. However, on some platforms, ethernet switches
> > >>are configured in Multi chip addressing mode and available at MDIO
> > >>address 0x1.
> > >
> > >Hi Romain
> > >
> > >What branch is this against? net-next?
> >
> > Until last friday it was based onto next-20161216, I rebased onto
> > the torvalds's tree this morning (so ~4.10-pre-rc1).
>
> This patchset is 80% networking. So please see:
>
> Documentation/networking/netdev-FAQ.txt
>
> > Oh, it's already fixed, good. I did not see this patch :)
> >
> > >
> > >It would be nicer to use Volodymyr, since it has been reviewed.
> >
> >
> > As the fix is already there, I will use it, sure.
>
> Im not sure what happened to it. It might of fallen through the
> cracks. Lets see what Volodymyr says. It might need resubmitting once
> netdev reopens.
>
> Andrew
>
[-- Attachment #2: Type: text/html, Size: 2034 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin
From: Romain Perier @ 2016-12-19 15:03 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, Jason Cooper,
Pawel Moll, Vivien Didelot, netdev, Ian Campbell, Nadav Haklai,
devicetree, Rob Herring, Kumar Gala, Gregory Clement,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161219155819.4cdf0f40@free-electrons.com>
Hi,
Le 19/12/2016 à 15:58, Thomas Petazzoni a écrit :
> Hello,
>
> On Mon, 19 Dec 2016 15:16:09 +0100, Romain Perier wrote:
>> This defines and enables the Marvell ethernet switch MVE886341 on the
>> Marvell EXPRESSObin board.
>>
>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>
> I didn't want to make this (silly) comment but since you got another
> comment that will let you send a new iteration, here is my silly
> comment: the name of the board is EspressoBin, not ExpressoBin.
>
> Thanks!
>
> Thomas
>
Ack for the two feedback (I don't know why I have replaced "es" by "ex"
:/, anyway...)
Thanks,
Romain
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Andrew Lunn @ 2016-12-19 15:00 UTC (permalink / raw)
To: Romain Perier
Cc: Volodymyr Bendiuga, Vivien Didelot, Florian Fainelli,
Jason Cooper, Sebastian Hesselbarth, Gregory Clement, netdev,
devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, linux-arm-kernel, Thomas Petazzoni, Nadav Haklai
In-Reply-To: <292d4f15-c58b-3565-26ec-98a025b6adb3@free-electrons.com>
On Mon, Dec 19, 2016 at 03:56:34PM +0100, Romain Perier wrote:
> Hi Andrew,
>
> Le 19/12/2016 à 15:38, Andrew Lunn a écrit :
> >On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
> >>Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
> >>zero of sw_addr is 0x1. However, on some platforms, ethernet switches
> >>are configured in Multi chip addressing mode and available at MDIO
> >>address 0x1.
> >
> >Hi Romain
> >
> >What branch is this against? net-next?
>
> Until last friday it was based onto next-20161216, I rebased onto
> the torvalds's tree this morning (so ~4.10-pre-rc1).
This patchset is 80% networking. So please see:
Documentation/networking/netdev-FAQ.txt
> Oh, it's already fixed, good. I did not see this patch :)
>
> >
> >It would be nicer to use Volodymyr, since it has been reviewed.
>
>
> As the fix is already there, I will use it, sure.
Im not sure what happened to it. It might of fallen through the
cracks. Lets see what Volodymyr says. It might need resubmitting once
netdev reopens.
Andrew
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: marvell: Add ethernet switch definition for the EXPRESSObin
From: Thomas Petazzoni @ 2016-12-19 14:58 UTC (permalink / raw)
To: Romain Perier
Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, Jason Cooper,
Pawel Moll, Vivien Didelot, netdev, Ian Campbell, Nadav Haklai,
devicetree, Rob Herring, Kumar Gala, Gregory Clement,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161219141610.30934-5-romain.perier@free-electrons.com>
Hello,
On Mon, 19 Dec 2016 15:16:09 +0100, Romain Perier wrote:
> This defines and enables the Marvell ethernet switch MVE886341 on the
> Marvell EXPRESSObin board.
>
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
I didn't want to make this (silly) comment but since you got another
comment that will let you send a new iteration, here is my silly
comment: the name of the board is EspressoBin, not ExpressoBin.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 3/4] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141
From: Romain Perier @ 2016-12-19 14:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: Mark Rutland, devicetree, Florian Fainelli, Jason Cooper,
Pawel Moll, Vivien Didelot, netdev, Ian Campbell, Nadav Haklai,
Rob Herring, Kumar Gala, Gregory Clement, Thomas Petazzoni,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161219145227.GC10048@lunn.ch>
Hi Andrew,
Le 19/12/2016 à 15:52, Andrew Lunn a écrit :
> Hi Romain
>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 76d944e..72ba24b 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -4086,6 +4086,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>> .ops = &mv88e6321_ops,
>> },
>>
>> + [MV88E6341] = {
>> + .prod_num = PORT_SWITCH_ID_PROD_NUM_6341,
>> + .family = MV88E6XXX_FAMILY_6352,
>> + .name = "Marvell 88E6341",
>> + .num_databases = 4096,
>> + .num_ports = 6,
>> + .port_base_addr = 0x10,
>> + .global1_addr = 0x1b,
>> + .age_time_coeff = 15000,
>> + .tag_protocol = DSA_TAG_PROTO_EDSA,
>> + .flags = MV88E6XXX_FLAGS_FAMILY_6352,
>> + .ops = &mv88e6352_ops,
>
> Even if it i 100% compatible with the 6532, you should still add an
> ops structure for it. All chips have their own, even when the are
> exactly the same as other in the family.
Ack, I will fix this.
>
>> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> @@ -86,6 +86,7 @@
>> #define PORT_SWITCH_ID_PROD_NUM_6097 0x099
>> #define PORT_SWITCH_ID_PROD_NUM_6131 0x106
>> #define PORT_SWITCH_ID_PROD_NUM_6320 0x115
>> +#define PORT_SWITCH_ID_PROD_NUM_6341 0x340
>> #define PORT_SWITCH_ID_PROD_NUM_6123 0x121
>> #define PORT_SWITCH_ID_PROD_NUM_6161 0x161
>
> Ah, err..
>
> These should be in numerical order of the macro.
> PORT_SWITCH_ID_PROD_NUM_6320 is however in the wrong place. Please
> put the 6341 after the 6321.
good catch, ok.
Thanks,
Romain
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: mv88e6xxx: Allow mv88e6xxx_smi_init() to be used at address 0x1
From: Romain Perier @ 2016-12-19 14:56 UTC (permalink / raw)
To: Andrew Lunn, Volodymyr Bendiuga
Cc: Mark Rutland, devicetree, Florian Fainelli, Jason Cooper,
Pawel Moll, Vivien Didelot, netdev, Ian Campbell, Nadav Haklai,
Rob Herring, Kumar Gala, Gregory Clement, Thomas Petazzoni,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161219143848.GA10048@lunn.ch>
Hi Andrew,
Le 19/12/2016 à 15:38, Andrew Lunn a écrit :
> On Mon, Dec 19, 2016 at 03:16:06PM +0100, Romain Perier wrote:
>> Currently, the function mv88e6xxx_smi_init() returns -EINVAL if the bit
>> zero of sw_addr is 0x1. However, on some platforms, ethernet switches
>> are configured in Multi chip addressing mode and available at MDIO
>> address 0x1.
>
> Hi Romain
>
> What branch is this against? net-next?
Until last friday it was based onto next-20161216, I rebased onto the
torvalds's tree this morning (so ~4.10-pre-rc1).
>
> Please see:
>
> https://www.spinics.net/lists/netdev/msg409156.html
Oh, it's already fixed, good. I did not see this patch :)
>
> It would be nicer to use Volodymyr, since it has been reviewed.
As the fix is already there, I will use it, sure.
Thanks,
Romain
^ permalink raw reply
* Re: [PATCH 2/4] net: dsa: mv88e6xxx: Don't forbid MDIO I/Os for PHY addr >= num_of_ports
From: Andrew Lunn @ 2016-12-19 14:56 UTC (permalink / raw)
To: Romain Perier
Cc: Mark Rutland, devicetree, Florian Fainelli, Jason Cooper,
Pawel Moll, Vivien Didelot, netdev, Ian Campbell, Nadav Haklai,
Rob Herring, Kumar Gala, Gregory Clement, Thomas Petazzoni,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161219141610.30934-3-romain.perier@free-electrons.com>
On Mon, Dec 19, 2016 at 03:16:07PM +0100, Romain Perier wrote:
> Some Marvell ethernet switches have internal ethernet transceivers with
> hardcoded phy addresses. These addresses can be grearer than the number
> of ports or its value might be different than the associated port number.
> This is for example the case for MV88E6341 that has 6 ports and internal
> Port 1 to Port4 PHYs mapped at SMI addresses from 0x11 to 0x14.
>
> This commits fixes the issue by removing the condition in MDIO callbacks.
>
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
So it is not quite compatible with the 6352. Thanks Marvell :-(
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 3/4] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341/88E6141
From: Andrew Lunn @ 2016-12-19 14:52 UTC (permalink / raw)
To: Romain Perier
Cc: Vivien Didelot, Florian Fainelli, Jason Cooper,
Sebastian Hesselbarth, Gregory Clement, netdev, devicetree,
Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel, Thomas Petazzoni, Nadav Haklai
In-Reply-To: <20161219141610.30934-4-romain.perier@free-electrons.com>
Hi Romain
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 76d944e..72ba24b 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4086,6 +4086,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
> .ops = &mv88e6321_ops,
> },
>
> + [MV88E6341] = {
> + .prod_num = PORT_SWITCH_ID_PROD_NUM_6341,
> + .family = MV88E6XXX_FAMILY_6352,
> + .name = "Marvell 88E6341",
> + .num_databases = 4096,
> + .num_ports = 6,
> + .port_base_addr = 0x10,
> + .global1_addr = 0x1b,
> + .age_time_coeff = 15000,
> + .tag_protocol = DSA_TAG_PROTO_EDSA,
> + .flags = MV88E6XXX_FLAGS_FAMILY_6352,
> + .ops = &mv88e6352_ops,
Even if it i 100% compatible with the 6532, you should still add an
ops structure for it. All chips have their own, even when the are
exactly the same as other in the family.
> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> @@ -86,6 +86,7 @@
> #define PORT_SWITCH_ID_PROD_NUM_6097 0x099
> #define PORT_SWITCH_ID_PROD_NUM_6131 0x106
> #define PORT_SWITCH_ID_PROD_NUM_6320 0x115
> +#define PORT_SWITCH_ID_PROD_NUM_6341 0x340
> #define PORT_SWITCH_ID_PROD_NUM_6123 0x121
> #define PORT_SWITCH_ID_PROD_NUM_6161 0x161
Ah, err..
These should be in numerical order of the macro.
PORT_SWITCH_ID_PROD_NUM_6320 is however in the wrong place. Please
put the 6341 after the 6321.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst
From: Christian Lamparter @ 2016-12-19 14:49 UTC (permalink / raw)
To: John Youn
Cc: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <dab2e32a-1bd0-2aa5-5a7a-61f2201786b4-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hello John, hello Felipe
On Monday, November 28, 2016 7:32:20 PM CET John Youn wrote:
> On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> >> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> >>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>>>> Also, perhaps you should allow that the compatible string can define the
> >>>>>> default.
> >>>>>>
> >>>>> I hoped you would say that :).
> >>>>>
> >>>>> I've attached a patch (on top of John Youn changes) [...]
> >>>>> ---
> >>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> >>>>> [...]
> >>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>>>> +/* [...] */
> >>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>>>> + {
> >>>>> + .compatible = "amcc,dwc-otg",
> >>>>> + .data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>>>> + },
> >>>>> +};
> >> [...]
> >>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >>>>> ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
> >>>>> if (ret < 0) {
> >>>>> + const struct of_device_id *match;
> >>>>> +
> >>>>> + match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>>>> + if (match)
> >>>>> + ret = (int)match->data;
> >>>>> +
> >> [...]
> >>>> I'd prefer if you use the binding which requires no extra code in
> >>>> dwc2.
> >>> I'm fine with either option. However it think that this would require
> >>> that either Mark or Rob would allow an exception to the "keep existing
> >>> dts the way they are) and ack the following change to the canyonlands.dts.
> >> [...]
>
> Ok thanks for clearing that up. I understand.
>
> For now we can just set the property to "INCR16" based on the
> compatible string. Perhaps in the future do this from a glue-layer
> driver which binds to all compatible strings other than "snps,dwc2".
>
> I won't be able to do anything with this until next week though.
Ok, I think enough time has passed. I would like to see this
patch series (v3 [0]) being queued for 4.11+ together with
"usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg" [1].
Felipe, if you want I can resend the series and add the
"amcc,dwc-otg" patch to it as well. Just let me know what you
prefer here.
Regards,
Christian
[0] <https://www.mail-archive.com/linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg83401.html>
[1] <https://www.spinics.net/lists/linux-usb/msg149663.html>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 9/9] ARM: dts: dra7: Add an empty chosen node to top level DTSI
From: Javier Martinez Canillas @ 2016-12-19 14:44 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Rutland, devicetree, Tony Lindgren, Russell King,
Javier Martinez Canillas, Rob Herring, Benoît Cousson,
Pali Rohar, linux-omap, linux-arm-kernel
In-Reply-To: <1482158681-4530-1-git-send-email-javier@osg.samsung.com>
Commit 55871eb6e2cc ("ARM: dts: dra7: Remove skeleton.dtsi usage")
removed the skeleton.dtsi usage since we want to get rid of it.
But this can cause issues when booting a kernel with a boot-loader
that doesn't create a chosen node if this isn't present in the DTB
since the decompressor relies on a pre-existing chosen node to be
available to insert the command line and merge other ATAGS info.
Fixes: 55871eb6e2cc ("ARM: dts: dra7: Remove skeleton.dtsi usage")
Reported-by: Pali Rohar <pali.rohar@gmail.com>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
arch/arm/boot/dts/dra7.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index addb7530cfbe..1faf24acd521 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -18,6 +18,7 @@
compatible = "ti,dra7xx";
interrupt-parent = <&crossbar_mpu>;
+ chosen { };
aliases {
i2c0 = &i2c1;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox