* Re: [PATCH v11 5/7] overlay: Documentation for the overlay sugar syntax
From: David Gibson @ 2016-11-30 0:39 UTC (permalink / raw)
To: Frank Rowand
Cc: Pantelis Antoniou, Jon Loeliger, Grant Likely, Rob Herring,
Jan Luebbe, Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <583DB09C.7060105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]
On Tue, Nov 29, 2016 at 08:45:16AM -0800, Frank Rowand wrote:
> On 11/28/16 21:10, David Gibson wrote:
> > On Mon, Nov 28, 2016 at 08:36:07PM -0800, Frank Rowand wrote:
> >> On 11/28/16 19:10, David Gibson wrote:
> >>> On Mon, Nov 28, 2016 at 06:05:39PM +0200, Pantelis Antoniou wrote:
> >>>> There exists a syntactic sugar version of overlays which
> >>>> make them simpler to write for the trivial case of a single target.
> >>
> >> It also works for multiple targets. (See the example I provided in
> >> my comment to v10.)
> >>
> >>
> >>>>
> >>>> Document it in the device tree object internals.
> >>>>
> >>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >>>
> >>> I'm with Frank that I think this, rather than being regarded mere
> >>> syntactic sugar, should be considered the primary way of describing
> >>> overlays.
> >>>
> >>> Obviously we need to support the fully written out version as well.
> >>
> >> If we need to support the fully written out version, can we make that
> >> a discouraged, non-preferred method? Maybe require an option to
> >> enable compiling this style of dts?
> >
> > Yeah. To avoid further proliferation of options, I'm thinking a
> > single "backwards compat" option which would:
> > - Use the dtb magic instead of dtb magic
> > - Disable checks which would reject explicit creation of
> > __overlay__ / __symbols__ / __fixups__ nodes
> > - Anything other special behaviour we need
> >
> >> I can imagine some reasons to support the fully written out version,
> >> but can we document what those reasons are?
> >
> > I believe the main one is the dts files in this format out in the
> > field. Mind you, I guess we're already requiring them to tweak how
> > they declare the /plugin/ option.
>
> It might be easy to write a program that transforms the expanded
> format to the simple format. I'll try to make some time to see
> how difficult it is. The transformation is relatively easy to
> do manually, but I don't know how many dts files would need to
> be converted.
It's not totally trivial, because such a program would basically need
a full dts parser. But.. if we change the dtc internals to work with
a list of overlays rather than a single tree, there's a relatively
obvious path to it: we can implement parsing a dtbo input into a set
of fragments rather than an assembled overlay and dts output in
fragment form. Then converting from old-dts to dtb and back to dts
would pretty much do what's needed. At the cost of losing comments,
though :/.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3 0/4] Fix OdroidC2 Gigabit Tx link issue
From: David Miller @ 2016-11-30 0:38 UTC (permalink / raw)
To: jbrunet-rdvid1DuHRBWk0Htik3J/w
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, carlo-KA+7E9HrN00dnm+yROfE0A,
khilman-rdvid1DuHRBWk0Htik3J/w, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o,
martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
neolynx-Re5JQEeQqe8AvxtiuMwx3w, andrew-g2DYL2Zd6BY,
narmstrong-rdvid1DuHRBWk0Htik3J/w,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480326409-25419-1-git-send-email-jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
From: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Date: Mon, 28 Nov 2016 10:46:45 +0100
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> The platform seems to enter LPI on the Rx path too often while performing
> relatively high TX transfer. This eventually break the link (both Tx and
> Rx), and require to bring the interface down and up again to get the Rx
> path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
>
> The patchset adds options in the generic phy driver to disable EEE
> advertisement, through device tree. The way it is done is very similar
> to the handling of the max-speed property.
Patches 1-3 applied to net-next, thanks.
--
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 v11 4/7] tests: Add overlay tests
From: David Gibson @ 2016-11-30 0:35 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Jon Loeliger, Grant Likely, Frank Rowand, Rob Herring, Jan Luebbe,
Sascha Hauer, Phil Elwell, Simon Glass, Maxime Ripard,
Thomas Petazzoni, Boris Brezillon, Antoine Tenart, Stephen Boyd,
Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <17F182F7-CDF7-4052-86C2-B40505706ED4-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 10918 bytes --]
On Tue, Nov 29, 2016 at 01:11:43PM +0200, Pantelis Antoniou wrote:
> Hi David,
>
> > On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> >> Add a number of tests for dynamic objects/overlays.
> >>
> >> Re-use the original test by moving the contents to a .dtsi include
> >>
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> ---
> >> tests/overlay_overlay_dtc.dts | 76 +----------------------------------
> >> tests/overlay_overlay_dtc.dtsi | 83 +++++++++++++++++++++++++++++++++++++++
> >> tests/overlay_overlay_new_dtc.dts | 11 ++++++
> >> tests/overlay_overlay_simple.dts | 12 ++++++
> >> tests/run_tests.sh | 41 +++++++++++++++++++
> >> 5 files changed, 148 insertions(+), 75 deletions(-)
> >> create mode 100644 tests/overlay_overlay_dtc.dtsi
> >> create mode 100644 tests/overlay_overlay_new_dtc.dts
> >> create mode 100644 tests/overlay_overlay_simple.dts
> >>
> >> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> >> index 30d2362..ca943ea 100644
> >> --- a/tests/overlay_overlay_dtc.dts
> >> +++ b/tests/overlay_overlay_dtc.dts
> >> @@ -8,78 +8,4 @@
> >> /dts-v1/;
> >> /plugin/;
> >>
> >> -/ {
> >> - /* Test that we can change an int by another */
> >> - fragment@0 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-int-property = <43>;
> >> - };
> >> - };
> >> -
> >> - /* Test that we can replace a string by a longer one */
> >> - fragment@1 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-str-property = "foobar";
> >> - };
> >> - };
> >> -
> >> - /* Test that we add a new property */
> >> - fragment@2 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-str-property-2 = "foobar2";
> >> - };
> >> - };
> >> -
> >> - /* Test that we add a new node (by phandle) */
> >> - fragment@3 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - new-node {
> >> - new-property;
> >> - };
> >> - };
> >> - };
> >> -
> >> - fragment@5 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - local: new-local-node {
> >> - new-property;
> >> - };
> >> - };
> >> - };
> >> -
> >> - fragment@6 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-phandle = <&test>, <&local>;
> >> - };
> >> - };
> >> -
> >> - fragment@7 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-several-phandle = <&local>, <&local>;
> >> - };
> >> - };
> >> -
> >> - fragment@8 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - sub-test-node {
> >> - new-sub-test-property;
> >> - };
> >> - };
> >> - };
> >> -};
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >
> > Don't duplicate this, just replace it with the new style. This only
> > existed as essentially documentation for the libfdt overlay
> > application stuff. Since the new dtc won't support the old tag
> > format, there's no point having a test for it.
>
> The parser now handles both tag formats just fine. I could remove support
> for it if you’re willing to tackle the flak.
Oh, sorry, I missed that.
I'd suggest having that controlled by the same "backwards compat"
option to control magic number and other things. Although changing
parser behaviour based on flags can get fiddly.
>
> >> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> >> new file mode 100644
> >> index 0000000..8ea8d5d
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_dtc.dtsi
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/ {
> >> + /* Test that we can change an int by another */
> >> + fragment@0 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-int-property = <43>;
> >> + };
> >> + };
> >> +
> >> + /* Test that we can replace a string by a longer one */
> >> + fragment@1 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-str-property = "foobar";
> >> + };
> >> + };
> >> +
> >> + /* Test that we add a new property */
> >> + fragment@2 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-str-property-2 = "foobar2";
> >> + };
> >> + };
> >> +
> >> + /* Test that we add a new node (by phandle) */
> >> + fragment@3 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + new-node {
> >> + new-property;
> >> + };
> >> + };
> >> + };
> >> +
> >> + fragment@5 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + local: new-local-node {
> >> + new-property;
> >> + };
> >> + };
> >> + };
> >> +
> >> + fragment@6 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-phandle = <&test>, <&local>;
> >> + };
> >> + };
> >> +
> >> + fragment@7 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-several-phandle = <&local>, <&local>;
> >> + };
> >> + };
> >> +
> >> + fragment@8 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + sub-test-node {
> >> + new-sub-test-property;
> >> + };
> >> + };
> >> + };
> >> +};
> >> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> >> new file mode 100644
> >> index 0000000..14d3f54
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_new_dtc.dts
> >> @@ -0,0 +1,11 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/ /plugin/;
> >> +
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> >> new file mode 100644
> >> index 0000000..8657e1e
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_simple.dts
> >> @@ -0,0 +1,12 @@
> >> +/*
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/plugin/;
> >> +
> >> +&test {
> >> + test-int-property = <43>;
> >> +};
> >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> >> index e4139dd..74af0ff 100755
> >> --- a/tests/run_tests.sh
> >> +++ b/tests/run_tests.sh
> >> @@ -181,6 +181,47 @@ overlay_tests () {
> >> run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
> >> run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
> >> run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> >> +
> >> + # new /plugin/ format
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
> >
> > Looks like you're mixing tabs and spaces here. I don't really mind
> > which, but keep it consistent at least at the same indentation level.
> >
>
> Oh, sorry, I use tabs but this sections has spaces… Will fix.
Well, it's more tha the first line in the block seems to be using
spaces, then the rest using tabs.
>
> >> + # test new magic option
> >> + run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> >> +
> >> + # test plugin source to dtb and back
> >> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> >> + run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> >> +
> >> + # test plugin source to dtb and back (with new magic)
> >> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> >> + run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> >> +
> >> + # test plugin auto-generation without using -@
> >> + run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> >> +
> >> + # Test suppression of fixups
> >> + run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> >> +
> >> + # Test generation of aliases insted of symbols
> >> + run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
> >> fi
> >>
> >> # Bad fixup tests
> >
>
> Regards
>
> — Pantelis
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v4 4/4] [media] dt-bindings: add TI VPIF documentation
From: Kevin Hilman @ 2016-11-29 23:57 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus, linux-arm-kernel,
Sekhar Nori, Rob Herring, devicetree
In-Reply-To: <20161129235712.29846-1-khilman@baylibre.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
.../devicetree/bindings/media/ti,da850-vpif.txt | 67 ++++++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
new file mode 100644
index 000000000000..fa06dfdb6898
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,da850-vpif.txt
@@ -0,0 +1,67 @@
+Texas Instruments VPIF
+----------------------
+
+The TI Video Port InterFace (VPIF) is the primary component for video
+capture and display on the DA850/AM18x family of TI DaVinci/Sitara
+SoCs.
+
+TI Document reference: SPRUH82C, Chapter 35
+http://www.ti.com/lit/pdf/spruh82
+
+Required properties:
+- compatible: must be "ti,da850-vpif"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the VPIF
+
+Video Capture:
+
+VPIF has a 16-bit parallel bus input, supporting 2 8-bit channels or a
+single 16-bit channel. It should contain at least one port child node
+with child 'endpoint' node. Please refer to the bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example using 2 8-bit input channels, one of which is connected to an
+I2C-connected TVP5147 decoder:
+
+ vpif: vpif@217000 {
+ compatible = "ti,da850-vpif";
+ reg = <0x217000 0x1000>;
+ interrupts = <92>;
+
+ port {
+ vpif_ch0: endpoint@0 {
+ reg = <0>;
+ bus-width = <8>;
+ remote-endpoint = <&composite>;
+ };
+
+ vpif_ch1: endpoint@1 {
+ reg = <1>;
+ bus-width = <8>;
+ data-shift = <8>;
+ };
+ };
+ };
+
+[ ... ]
+
+&i2c0 {
+
+ tvp5147@5d {
+ compatible = "ti,tvp5147";
+ reg = <0x5d>;
+ status = "okay";
+
+ port {
+ composite: endpoint {
+ hsync-active = <1>;
+ vsync-active = <1>;
+ pclk-sample = <0>;
+
+ /* VPIF channel 0 (lower 8-bits) */
+ remote-endpoint = <&vpif_ch0>;
+ bus-width = <8>;
+ };
+ };
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v4 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Kevin Hilman @ 2016-11-29 23:57 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sekhar Nori,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129235712.29846-1-khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Allow getting of subdevs from DT ports and endpoints.
The _get_pdata() function was larely inspired by (i.e. stolen from)
am437x-vpfe.c
Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
drivers/media/platform/davinci/vpif_capture.c | 138 +++++++++++++++++++++++++-
include/media/davinci/vpif_types.h | 9 +-
2 files changed, 141 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index a83df07e4051..4e363da2c21f 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -26,6 +26,8 @@
#include <linux/slab.h>
#include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/i2c/tvp514x.h>
#include "vpif.h"
#include "vpif_capture.h"
@@ -651,6 +653,10 @@ static int vpif_input_to_subdev(
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
+ if (!chan_cfg)
+ return -1;
+ if (input_index >= chan_cfg->input_count)
+ return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -658,7 +664,7 @@ static int vpif_input_to_subdev(
/* loop through the sub device list to get the sub device info */
for (i = 0; i < vpif_cfg->subdev_count; i++) {
subdev_info = &vpif_cfg->subdev_info[i];
- if (!strcmp(subdev_info->name, subdev_name))
+ if (subdev_info && !strcmp(subdev_info->name, subdev_name))
return i;
}
return -1;
@@ -1328,6 +1334,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
{
int i;
+ for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
+ struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
+ const struct device_node *node = _asd->match.of.node;
+
+ if (node == subdev->of_node) {
+ vpif_obj.sd[i] = subdev;
+ vpif_obj.config->chan_config->inputs[i].subdev_name =
+ (char *)subdev->of_node->full_name;
+ vpif_dbg(2, debug,
+ "%s: setting input %d subdev_name = %s\n",
+ __func__, i, subdev->of_node->full_name);
+ return 0;
+ }
+ }
+
for (i = 0; i < vpif_obj.config->subdev_count; i++)
if (!strcmp(vpif_obj.config->subdev_info[i].name,
subdev->name)) {
@@ -1423,6 +1444,118 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
return vpif_probe_complete();
}
+static struct vpif_capture_config *
+vpif_capture_get_pdata(struct platform_device *pdev)
+{
+ struct device_node *endpoint = NULL;
+ struct v4l2_of_endpoint bus_cfg;
+ struct vpif_capture_config *pdata;
+ struct vpif_subdev_info *sdinfo;
+ struct vpif_capture_chan_config *chan;
+ unsigned int i;
+
+ dev_dbg(&pdev->dev, "vpif_get_pdata\n");
+
+ /*
+ * DT boot: OF node from parent device contains
+ * video ports & endpoints data.
+ */
+ if (pdev->dev.parent && pdev->dev.parent->of_node)
+ pdev->dev.of_node = pdev->dev.parent->of_node;
+ if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+ return pdev->dev.platform_data;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+ pdata->subdev_info =
+ devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
+ VPIF_CAPTURE_NUM_CHANNELS, GFP_KERNEL);
+
+ if (!pdata->subdev_info)
+ return NULL;
+
+ for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
+ struct device_node *rem;
+ unsigned int flags;
+ int err;
+
+ endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
+ endpoint);
+ if (!endpoint)
+ break;
+
+ sdinfo = &pdata->subdev_info[i];
+ chan = &pdata->chan_config[i];
+ chan->inputs = devm_kzalloc(&pdev->dev,
+ sizeof(*chan->inputs) *
+ VPIF_CAPTURE_NUM_CHANNELS,
+ GFP_KERNEL);
+
+ chan->input_count++;
+ chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
+ chan->inputs[i].input.std = V4L2_STD_ALL;
+ chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
+
+ /*
+ * FIXME: need a new property for input/output routing?
+ * All known boards are using ch0:composite ch1: s-video.
+ */
+ if (i == 0)
+ chan->inputs[i].input_route = INPUT_CVBS_VI2B;
+ else
+ chan->inputs[i].input_route = INPUT_SVIDEO_VI2C_VI1C;
+ chan->inputs[i].output_route = OUTPUT_10BIT_422_EMBEDDED_SYNC;
+
+ err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+ if (err) {
+ dev_err(&pdev->dev, "Could not parse the endpoint\n");
+ goto done;
+ }
+ dev_dbg(&pdev->dev, "Endpoint %s, bus_width = %d\n",
+ endpoint->full_name, bus_cfg.bus.parallel.bus_width);
+ flags = bus_cfg.bus.parallel.flags;
+
+ if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+ chan->vpif_if.hd_pol = 1;
+
+ if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+ chan->vpif_if.vd_pol = 1;
+
+ chan->vpif_if.if_type = VPIF_IF_BT656;
+ rem = of_graph_get_remote_port_parent(endpoint);
+ if (!rem) {
+ dev_dbg(&pdev->dev, "Remote device at %s not found\n",
+ endpoint->full_name);
+ goto done;
+ }
+
+ dev_dbg(&pdev->dev, "Remote device %s, %s found\n",
+ rem->name, rem->full_name);
+ sdinfo->name = rem->full_name;
+
+ pdata->asd[i] = devm_kzalloc(&pdev->dev,
+ sizeof(struct v4l2_async_subdev),
+ GFP_KERNEL);
+ if (!pdata->asd[i]) {
+ of_node_put(rem);
+ pdata = NULL;
+ goto done;
+ }
+
+ pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
+ pdata->asd[i]->match.of.node = rem;
+ of_node_put(rem);
+ }
+
+done:
+ pdata->asd_sizes[0] = i;
+ pdata->subdev_count = i;
+ pdata->card_name = "DA850/OMAP-L138 Video Capture";
+
+ return pdata;
+}
+
/**
* vpif_probe : This function probes the vpif capture driver
* @pdev: platform device pointer
@@ -1439,6 +1572,7 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ pdev->dev.platform_data = vpif_capture_get_pdata(pdev);
if (!pdev->dev.platform_data) {
dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
return -EINVAL;
@@ -1481,7 +1615,7 @@ static __init int vpif_probe(struct platform_device *pdev)
goto vpif_unregister;
}
- if (!vpif_obj.config->asd_sizes) {
+ if (!vpif_obj.config->asd_sizes[0]) {
i2c_adap = i2c_get_adapter(1);
for (i = 0; i < subdev_count; i++) {
subdevdata = &vpif_obj.config->subdev_info[i];
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 3cb1704a0650..4ee3b41975db 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -65,14 +65,14 @@ struct vpif_display_config {
struct vpif_input {
struct v4l2_input input;
- const char *subdev_name;
+ char *subdev_name;
u32 input_route;
u32 output_route;
};
struct vpif_capture_chan_config {
struct vpif_interface vpif_if;
- const struct vpif_input *inputs;
+ struct vpif_input *inputs;
int input_count;
};
@@ -83,7 +83,8 @@ struct vpif_capture_config {
struct vpif_subdev_info *subdev_info;
int subdev_count;
const char *card_name;
- struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
- int *asd_sizes; /* 0-terminated array of asd group sizes */
+
+ struct v4l2_async_subdev *asd[VPIF_CAPTURE_MAX_CHANNELS];
+ int asd_sizes[VPIF_CAPTURE_MAX_CHANNELS];
};
#endif /* _VPIF_TYPES_H */
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 2/4] [media] davinci: VPIF: add basic support for DT init
From: Kevin Hilman @ 2016-11-29 23:57 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sekhar Nori,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129235712.29846-1-khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Add basic support for initialization via DT.
Because existing capture and display devices are implemented as separate
platform_devices, and in order to preserve that legacy support, during
DT boot, manually create capture and display platform devices to
initialize capture and display support.
Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
drivers/media/platform/davinci/vpif.c | 48 ++++++++++++++++++++++++++-
drivers/media/platform/davinci/vpif_capture.c | 6 ++++
drivers/media/platform/davinci/vpif_display.c | 6 ++++
3 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..528b30d52208 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -420,7 +420,8 @@ EXPORT_SYMBOL(vpif_channel_getfid);
static int vpif_probe(struct platform_device *pdev)
{
- static struct resource *res;
+ static struct resource *res, *res_irq;
+ struct platform_device *pdev_capture, *pdev_display;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
vpif_base = devm_ioremap_resource(&pdev->dev, res);
@@ -432,6 +433,42 @@ static int vpif_probe(struct platform_device *pdev)
spin_lock_init(&vpif_lock);
dev_info(&pdev->dev, "vpif probe success\n");
+
+ if (!pdev->dev.of_node)
+ return 0;
+
+ /*
+ * For DT platforms, manually create platform_devices for
+ * capture/display drivers.
+ */
+ res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (!res_irq) {
+ dev_warn(&pdev->dev, "Missing IRQ resource.\n");
+ return -EINVAL;
+ }
+
+ pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
+ GFP_KERNEL);
+ pdev_capture->name = "vpif_capture";
+ pdev_capture->id = -1;
+ pdev_capture->resource = res_irq;
+ pdev_capture->num_resources = 1;
+ pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
+ pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+ pdev_capture->dev.parent = &pdev->dev;
+ platform_device_register(pdev_capture);
+
+ pdev_display = devm_kzalloc(&pdev->dev, sizeof(*pdev_display),
+ GFP_KERNEL);
+ pdev_display->name = "vpif_display";
+ pdev_display->id = -1;
+ pdev_display->resource = res_irq;
+ pdev_display->num_resources = 1;
+ pdev_display->dev.dma_mask = pdev->dev.dma_mask;
+ pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+ pdev_display->dev.parent = &pdev->dev;
+ platform_device_register(pdev_display);
+
return 0;
}
@@ -464,8 +501,17 @@ static const struct dev_pm_ops vpif_pm = {
#define vpif_pm_ops NULL
#endif
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id vpif_of_match[] = {
+ { .compatible = "ti,da850-vpif", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, vpif_of_match);
+#endif
+
static struct platform_driver vpif_driver = {
.driver = {
+ .of_match_table = of_match_ptr(vpif_of_match),
.name = "vpif",
.pm = vpif_pm_ops,
},
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 9f8f41c0f251..a83df07e4051 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* global variables */
static struct vpif_device vpif_obj = { {NULL} };
@@ -1438,6 +1439,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 1/4] [media] davinci: vpif_capture: don't lock over s_stream
From: Kevin Hilman @ 2016-11-29 23:57 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sekhar Nori,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129235712.29846-1-khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Video capture subdevs may be over I2C and may sleep during xfer, so we
cannot do IRQ-disabled locking when calling the subdev.
Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
drivers/media/platform/davinci/vpif_capture.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..9f8f41c0f251 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -193,7 +193,10 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
}
}
+ spin_unlock_irqrestore(&common->irqlock, flags);
ret = v4l2_subdev_call(ch->sd, video, s_stream, 1);
+ spin_lock_irqsave(&common->irqlock, flags);
+
if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
vpif_dbg(1, debug, "stream on failed in subdev\n");
goto err;
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 0/4] davinci: VPIF: add DT support
From: Kevin Hilman @ 2016-11-29 23:57 UTC (permalink / raw)
To: linux-media-u79uwXL29TY76Z2rM5mHXA
Cc: Hans Verkuil, Laurent Pinchart, Sakari Ailus,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sekhar Nori,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA
Add DT support, including getting subdevs from DT ports/endpoints.
Tested video capture to memory on da850-lcdk board using composite
input.
Changes since v3:
- move to a single VPIF node, DT binding updated accordingly
- misc fixes/updates based on reviews from Sakari
Changes since v2:
- DT binding doc: fix example to use correct compatible
Changes since v1:
- more specific compatible strings, based on SoC: ti,da850-vpif*
- fix locking bug when unlocking over subdev s_stream
Kevin Hilman (4):
[media] davinci: vpif_capture: don't lock over s_stream
[media] davinci: VPIF: add basic support for DT init
[media] davinci: vpif_capture: get subdevs from DT
[media] dt-bindings: add TI VPIF documentation
.../devicetree/bindings/media/ti,da850-vpif.txt | 67 ++++++++++
drivers/media/platform/davinci/vpif.c | 48 ++++++-
drivers/media/platform/davinci/vpif_capture.c | 147 ++++++++++++++++++++-
drivers/media/platform/davinci/vpif_display.c | 6 +
include/media/davinci/vpif_types.h | 9 +-
5 files changed, 270 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
--
2.9.3
--
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 v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Jernej Skrabec @ 2016-11-29 23:24 UTC (permalink / raw)
To: linux-sunxi
Cc: jernej.skrabec-Re5JQEeQqe8AvxtiuMwx3w, moinejf-GANU6spQydw,
airlied-cv59FeDIM0c, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
kieran.bingham-ryLnwIuWjnjg/C1BVhZhaw,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <2256264.6LWGGhFAiO@avalon>
[-- Attachment #1.1: Type: text/plain, Size: 3278 bytes --]
Hi Laurent,
Dne torek, 29. november 2016 23.56.31 UTC+1 je oseba Laurent Pinchart
napisala:
>
> Hi Jernej,
>
> (CC'ing Kieran Bingham)
>
> On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote:
> > Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard
> napisala:
> > > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > >> This patchset series adds HDMI video support to the Allwinner
> > >> sun8i SoCs which include the display engine 2 (DE2).
> > >> The driver contains the code for the A83T and H3 SoCs, and
> > >> some H3 boards, but it could be used/extended for other SoCs
> > >> (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> > >
> > > Honestly, I'm getting a bit worried by the fact that you ignore
> > > reviews.
> > >
> > > On the important reviews that you got that are to be seen as major
> > >
> > > issues that block the inclusion, we have:
> > > - The fact that the HDMI driver is actually just a designware IP,
> > > and while you should use the driver that already exists, you just
> > > duplicated all that code.
> >
> > That might be hard thing to do. A83T fits perfectly, but H3 and newer
> SoCs
> > do not. They are using completely different HDMI phy. Decoupling
> controller
> > and phy code means rewritting a good portion of the code, unless some
> tricks
> > are applied, like calling phy function pointers, if they are defined.
>
> Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling
> the
> PHY configuration code for a Renesas SoC, that might be of interest to
> you.
>
Exactly. I'm developing only U-Boot driver, but Jean-Francois will probably
have more interest in this.
>
> > Register addresses also differ, but that can be easily solved by using
> > undocumented magic value to restore them.
>
> I love that :-)
>
>
Is it allowed to use magic number which was found in binary blob? I'm new in
all this.
> > > - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> > > graph to model the connection between the display engine and the
> > > TCON. Something that Laurent also pointed out in this version.
> > >
> > > - The fact that you ignored that you needed an HDMI connector node
> > > as a child of the HDMI controller. This has been reported by Rob
> > > (v6) and yet again in this version by Laurent.
> > >
> > > - And finally the fact that we can't have several display engine in
> > > parallel, if needs be. This has happened in the past already on
> > > Allwinner SoCs, so it's definitely something we should consider in
> > > the DT bindings, since we can't break them.
> > >
> > > Until those are fixed, I cannot see how this driver can be merged,
> > > unfortunately.
>
> --
> Regards,
>
> Laurent Pinchart
>
>
Best regards,
Jernej Škrabec
--
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.
[-- Attachment #1.2: Type: text/html, Size: 4232 bytes --]
^ permalink raw reply
* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Laurent Pinchart @ 2016-11-29 22:56 UTC (permalink / raw)
To: Jernej Skrabec
Cc: linux-sunxi, moinejf-GANU6spQydw, airlied-cv59FeDIM0c,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
kieran.bingham-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <8398357e-5c5e-4d76-9022-1c668aff5076-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org>
Hi Jernej,
(CC'ing Kieran Bingham)
On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote:
> Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard napisala:
> > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> >> This patchset series adds HDMI video support to the Allwinner
> >> sun8i SoCs which include the display engine 2 (DE2).
> >> The driver contains the code for the A83T and H3 SoCs, and
> >> some H3 boards, but it could be used/extended for other SoCs
> >> (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> >
> > Honestly, I'm getting a bit worried by the fact that you ignore
> > reviews.
> >
> > On the important reviews that you got that are to be seen as major
> >
> > issues that block the inclusion, we have:
> > - The fact that the HDMI driver is actually just a designware IP,
> > and while you should use the driver that already exists, you just
> > duplicated all that code.
>
> That might be hard thing to do. A83T fits perfectly, but H3 and newer SoCs
> do not. They are using completely different HDMI phy. Decoupling controller
> and phy code means rewritting a good portion of the code, unless some tricks
> are applied, like calling phy function pointers, if they are defined.
Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling the
PHY configuration code for a Renesas SoC, that might be of interest to you.
> Register addresses also differ, but that can be easily solved by using
> undocumented magic value to restore them.
I love that :-)
> > - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> > graph to model the connection between the display engine and the
> > TCON. Something that Laurent also pointed out in this version.
> >
> > - The fact that you ignored that you needed an HDMI connector node
> > as a child of the HDMI controller. This has been reported by Rob
> > (v6) and yet again in this version by Laurent.
> >
> > - And finally the fact that we can't have several display engine in
> > parallel, if needs be. This has happened in the past already on
> > Allwinner SoCs, so it's definitely something we should consider in
> > the DT bindings, since we can't break them.
> >
> > Until those are fixed, I cannot see how this driver can be merged,
> > unfortunately.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Jernej Skrabec @ 2016-11-29 22:47 UTC (permalink / raw)
To: linux-sunxi
Cc: moinejf-GANU6spQydw, airlied-cv59FeDIM0c,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20161129213650.uqcgekodq77wlmxs@lukather>
[-- Attachment #1.1: Type: text/plain, Size: 2581 bytes --]
Hi Maxime,
Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard napisala:
>
> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > This patchset series adds HDMI video support to the Allwinner
> > sun8i SoCs which include the display engine 2 (DE2).
> > The driver contains the code for the A83T and H3 SoCs, and
> > some H3 boards, but it could be used/extended for other SoCs
> > (A64, H2, H5) and boards (Banana PIs, Orange PIs).
>
> Honestly, I'm getting a bit worried by the fact that you ignore
> reviews.
>
> On the important reviews that you got that are to be seen as major
> issues that block the inclusion, we have:
> - The fact that the HDMI driver is actually just a designware IP,
> and while you should use the driver that already exists, you just
> duplicated all that code.
>
>
That might be hard thing to do. A83T fits perfectly, but H3 and newer SoCs
do
not. They are using completely different HDMI phy. Decoupling controller and
phy code means rewritting a good portion of the code, unless some tricks are
applied, like calling phy function pointers, if they are defined.
Register addresses also differ, but that can be easily solved by using
undocumented magic value to restore them.
> - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> graph to model the connection between the display engine and the
> TCON. Something that Laurent also pointed out in this version.
>
> - The fact that you ignored that you needed an HDMI connector node
> as a child of the HDMI controller. This has been reported by Rob
> (v6) and yet again in this version by Laurent.
>
> - And finally the fact that we can't have several display engine in
> parallel, if needs be. This has happened in the past already on
> Allwinner SoCs, so it's definitely something we should consider in
> the DT bindings, since we can't break them.
>
> Until those are fixed, I cannot see how this driver can be merged,
> unfortunately.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Best regards,
Jernej Škrabec
--
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.
[-- Attachment #1.2: Type: text/html, Size: 3602 bytes --]
^ permalink raw reply
* [PATCH v3 2/2] eeprom: Add IDT 89HPESx driver dts-binding file
From: Serge Semin @ 2016-11-29 22:27 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Serge Semin
In-Reply-To: <1480458434-22523-1-git-send-email-fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
IDT 89HPESx PCIe-switches exposes SMBus interface to have an access to
the device CSRs and EEPROM. So to properly utilize the interface
functionality, developer should declare a valid dts-file node, which
would refer to the corresponding 89HPESx device.
Signed-off-by: Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../devicetree/bindings/misc/idt_89hpesx.txt | 41 ++++++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
index 0000000..469cc93
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
@@ -0,0 +1,41 @@
+EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
+
+Required properties:
+ - compatible : should be "<manufacturer>,<type>"
+ Basically there is only one manufacturer: idt, but some
+ compatible devices may be produced in future. Following devices
+ are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
+ 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
+ 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
+ 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
+ 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
+ 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
+ 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
+ 89hpes64h16ag2;
+ 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
+ 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
+ 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
+ 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
+ 89hpes48t12, 89hpes48t12g2.
+ Current implementation of the driver doesn't have any device-
+ specific functionalities. But since each of them differs
+ by registers mapping, CSRs read/write restrictions can be
+ added in future.
+ - reg : I2C address of the IDT 89HPES device.
+
+Optional properties:
+ - read-only : Parameterless property disables writes to the EEPROM
+ - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus
+ (default value is 4096 bytes if option isn't specified)
+ - idt,eeaddr : Custom address of EEPROM device
+ (If not specified IDT 89HPESx device will try to communicate
+ with EEPROM sited by default address - 0x50)
+
+Example:
+ idt_pcie_sw@60 {
+ compatible = "idt,89hpes12nt3";
+ reg = <0x60>;
+ read-only;
+ idt,eesize = <65536>;
+ idt,eeaddr = <0x50>;
+ };
--
2.6.6
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2016-11-29 22:27 UTC (permalink / raw)
To: gregkh, srinivas.kandagatla, andrew, robh+dt, mark.rutland
Cc: Sergey.Semin, linux-kernel, devicetree, Serge Semin
In-Reply-To: <1480458434-22523-1-git-send-email-fancer.lancer@gmail.com>
This driver provides an access to EEPROM of IDT PCIe-switches. IDT PCIe-
switches expose a simple SMBus interface to perform IO-operations from/to
EEPROM, which is located at private (so called Master) SMBus. The driver
creates a simple binary sysfs-file to have an access to the EEPROM using
the SMBus-slave interface in the i2c-device susfs-directory.
Additionally IDT 89HPESx SMBus interface has an ability to read/write
values of device CSRs. This driver exposes debugfs-file to perform simple
IO-operations using that ability for just basic debug purpose.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
drivers/misc/eeprom/Kconfig | 10 +
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/idt_89hpesx.c | 1574 +++++++++++++++++++++++++++++++++++++
3 files changed, 1585 insertions(+)
create mode 100644 drivers/misc/eeprom/idt_89hpesx.c
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index c4e41c2..de58762 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -100,4 +100,14 @@ config EEPROM_DIGSY_MTC_CFG
If unsure, say N.
+config EEPROM_IDT_89HPESX
+ tristate "IDT 89HPESx PCIe-swtiches EEPROM / CSR support"
+ depends on I2C && SYSFS
+ help
+ Enable this driver to get read/write access to EEPROM / CSRs
+ over IDT PCIe-swtich i2c-slave interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called idt_89hpesx.
+
endmenu
diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
index fc1e81d..90a5262 100644
--- a/drivers/misc/eeprom/Makefile
+++ b/drivers/misc/eeprom/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
+obj-$(CONFIG_EEPROM_IDT_89HPESX) += idt_89hpesx.o
diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
index 0000000..a54c0f4
--- /dev/null
+++ b/drivers/misc/eeprom/idt_89hpesx.c
@@ -0,0 +1,1574 @@
+/*
+ * This file is provided under a GPLv2 license. When using or
+ * redistributing this file, you may do so under that license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (C) 2016 T-Platforms. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, it can be found <http://www.gnu.org/licenses/>.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * IDT PCIe-switch NTB Linux driver
+ *
+ * Contact Information:
+ * Serge Semin <fancer.lancer@gmail.com>, <Sergey.Semin@t-platforms.ru>
+ */
+/*
+ * NOTE of the IDT 89HPESx SMBus-slave interface driver
+ * This driver primarily is developed to have an access to EEPROM device of
+ * IDT PCIe-switches. IDT provides a simple SMBus interface to perform IO-
+ * operations from/to EEPROM, which is located at private (so called Master)
+ * SMBus of switches. Using that interface this the driver creates a simple
+ * binary sysfs-file in the device directory:
+ * /sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
+ * In case if read-only flag is specified in the dts-node of device desription,
+ * User-space applications won't be able to write to the EEPROM sysfs-node.
+ * Additionally IDT 89HPESx SMBus interface has an ability to write/read
+ * data of device CSRs. This driver exposes debugf-file to perform simple IO
+ * operations using that ability for just basic debug purpose. Particularly
+ * next file is created in the specific debugfs-directory:
+ * /sys/kernel/debug/idt_csr/
+ * Format of the debugfs-node is:
+ * $ cat /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
+ * <CSR address>:<CSR value>
+ * So reading the content of the file gives current CSR address and it value.
+ * If User-space application wishes to change current CSR address,
+ * it can just write a proper value to the sysfs-file:
+ * $ echo "<CSR address>" > /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>
+ * If it wants to change the CSR value as well, the format of the write
+ * operation is:
+ * $ echo "<CSR address>:<CSR value>" > \
+ * /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
+ * CSR address and value can be any of hexadecimal, decimal or octal format.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/debugfs.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/pci_ids.h>
+#include <linux/delay.h>
+
+#define IDT_NAME "89hpesx"
+#define IDT_89HPESX_DESC "IDT 89HPESx SMBus-slave interface driver"
+#define IDT_89HPESX_VER "1.0"
+
+MODULE_DESCRIPTION(IDT_89HPESX_DESC);
+MODULE_VERSION(IDT_89HPESX_VER);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("T-platforms");
+
+/*
+ * csr_dbgdir - CSR read/write operations Debugfs directory
+ */
+static struct dentry *csr_dbgdir;
+
+/*
+ * struct idt_89hpesx_dev - IDT 89HPESx device data structure
+ * @eesize: Size of EEPROM in bytes (calculated from "idt,eecompatible")
+ * @eero: EEPROM Read-only flag
+ * @eeaddr: EEPROM custom address
+ *
+ * @inieecmd: Initial cmd value for EEPROM read/write operations
+ * @inicsrcmd: Initial cmd value for CSR read/write operations
+ * @iniccode: Initialial command code value for IO-operations
+ *
+ * @csr: CSR address to perform read operation
+ *
+ * @smb_write: SMBus write method
+ * @smb_read: SMBus read method
+ * @smb_mtx: SMBus mutex
+ *
+ * @client: i2c client used to perform IO operations
+ *
+ * @ee_file: EEPROM read/write sysfs-file
+ * @csr_file: CSR read/write debugfs-node
+ */
+struct idt_smb_seq;
+struct idt_89hpesx_dev {
+ u32 eesize;
+ bool eero;
+ u8 eeaddr;
+
+ u8 inieecmd;
+ u8 inicsrcmd;
+ u8 iniccode;
+
+ atomic_t csr;
+
+ int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
+ int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
+ struct mutex smb_mtx;
+
+ struct i2c_client *client;
+
+ struct bin_attribute *ee_file;
+ struct dentry *csr_dir;
+ struct dentry *csr_file;
+};
+
+/*
+ * struct idt_smb_seq - sequence of data to be read/written from/to IDT 89HPESx
+ * @ccode: SMBus command code
+ * @bytecnt: Byte count of operation
+ * @data: Data to by written
+ */
+struct idt_smb_seq {
+ u8 ccode;
+ u8 bytecnt;
+ u8 *data;
+};
+
+/*
+ * struct idt_eeprom_seq - sequence of data to be read/written from/to EEPROM
+ * @cmd: Transaction CMD
+ * @eeaddr: EEPROM custom address
+ * @memaddr: Internal memory address of EEPROM
+ * @data: Data to be written at the memory address
+ */
+struct idt_eeprom_seq {
+ u8 cmd;
+ u8 eeaddr;
+ u16 memaddr;
+ u8 data;
+} __packed;
+
+/*
+ * struct idt_csr_seq - sequence of data to be read/written from/to CSR
+ * @cmd: Transaction CMD
+ * @csraddr: Internal IDT device CSR address
+ * @data: Data to be read/written from/to the CSR address
+ */
+struct idt_csr_seq {
+ u8 cmd;
+ u16 csraddr;
+ u32 data;
+} __packed;
+
+/*
+ * SMBus command code macros
+ * @CCODE_END: Indicates the end of transaction
+ * @CCODE_START: Indicates the start of transaction
+ * @CCODE_CSR: CSR read/write transaction
+ * @CCODE_EEPROM: EEPROM read/write transaction
+ * @CCODE_BYTE: Supplied data has BYTE length
+ * @CCODE_WORD: Supplied data has WORD length
+ * @CCODE_BLOCK: Supplied data has variable length passed in bytecnt
+ * byte right following CCODE byte
+ */
+#define CCODE_END ((u8)0x01)
+#define CCODE_START ((u8)0x02)
+#define CCODE_CSR ((u8)0x00)
+#define CCODE_EEPROM ((u8)0x04)
+#define CCODE_BYTE ((u8)0x00)
+#define CCODE_WORD ((u8)0x20)
+#define CCODE_BLOCK ((u8)0x40)
+#define CCODE_PEC ((u8)0x80)
+
+/*
+ * EEPROM command macros
+ * @EEPROM_OP_WRITE: EEPROM write operation
+ * @EEPROM_OP_READ: EEPROM read operation
+ * @EEPROM_USA: Use specified address of EEPROM
+ * @EEPROM_NAERR: EEPROM device is not ready to respond
+ * @EEPROM_LAERR: EEPROM arbitration loss error
+ * @EEPROM_MSS: EEPROM misplace start & stop bits error
+ * @EEPROM_WR_CNT: Bytes count to perform write operation
+ * @EEPROM_WRRD_CNT: Bytes count to write before reading
+ * @EEPROM_RD_CNT: Bytes count to perform read operation
+ * @EEPROM_DEF_SIZE: Fall back size of EEPROM
+ * @EEPROM_DEF_ADDR: Defatul EEPROM address
+ * @EEPROM_TOUT: Timeout before retry read operation if eeprom is busy
+ */
+#define EEPROM_OP_WRITE ((u8)0x00)
+#define EEPROM_OP_READ ((u8)0x01)
+#define EEPROM_USA ((u8)0x02)
+#define EEPROM_NAERR ((u8)0x08)
+#define EEPROM_LAERR ((u8)0x10)
+#define EEPROM_MSS ((u8)0x20)
+#define EEPROM_WR_CNT ((u8)5)
+#define EEPROM_WRRD_CNT ((u8)4)
+#define EEPROM_RD_CNT ((u8)5)
+#define EEPROM_DEF_SIZE ((u16)4096)
+#define EEPROM_DEF_ADDR ((u8)0x50)
+#define EEPROM_TOUT (100)
+
+/*
+ * CSR command macros
+ * @CSR_DWE: Enable all four bytes of the operation
+ * @CSR_OP_WRITE: CSR write operation
+ * @CSR_OP_READ: CSR read operation
+ * @CSR_RERR: Read operation error
+ * @CSR_WERR: Write operation error
+ * @CSR_WR_CNT: Bytes count to perform write operation
+ * @CSR_WRRD_CNT: Bytes count to write before reading
+ * @CSR_RD_CNT: Bytes count to perform read operation
+ * @CSR_MAX: Maximum CSR address
+ * @CSR_DEF: Default CSR address
+ * @CSR_REAL_ADDR: CSR real unshifted address
+ */
+#define CSR_DWE ((u8)0x0F)
+#define CSR_OP_WRITE ((u8)0x00)
+#define CSR_OP_READ ((u8)0x10)
+#define CSR_RERR ((u8)0x40)
+#define CSR_WERR ((u8)0x80)
+#define CSR_WR_CNT ((u8)7)
+#define CSR_WRRD_CNT ((u8)3)
+#define CSR_RD_CNT ((u8)7)
+#define CSR_MAX ((u32)0x3FFFF)
+#define CSR_DEF ((u16)0x0000)
+#define CSR_REAL_ADDR(val) ((unsigned int)val << 2)
+
+/*
+ * IDT 89HPESx basic register
+ * @IDT_VIDDID_CSR: PCIe VID and DID of IDT 89HPESx
+ * @IDT_VID_MASK: Mask of VID
+ */
+#define IDT_VIDDID_CSR ((u32)0x0000)
+#define IDT_VID_MASK ((u32)0xFFFF)
+
+/*
+ * IDT 89HPESx can send NACK when new command is sent before previous one
+ * fininshed execution. In this case driver retries operation
+ * certain times.
+ * @RETRY_CNT: Number of retries before giving up and fail
+ * @idt_smb_safe: Generate a retry loop on corresponding SMBus method
+ */
+#define RETRY_CNT (128)
+#define idt_smb_safe(ops, args...) ({ \
+ int __retry = RETRY_CNT; \
+ s32 __sts; \
+ do { \
+ __sts = i2c_smbus_ ## ops ## _data(args); \
+ } while (__retry-- && __sts < 0); \
+ __sts; \
+})
+
+/*===========================================================================
+ * i2c bus level IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_smb_write_byte() - SMBus write method when I2C_SMBUS_BYTE_DATA operation
+ * is only available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_byte(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx;
+
+ /* Loop over the supplied data sending byte one-by-one */
+ for (idx = 0; idx < seq->bytecnt; idx++) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == seq->bytecnt - 1)
+ ccode |= CCODE_END;
+
+ /* Send data to the device */
+ sts = idt_smb_safe(write_byte, pdev->client, ccode,
+ seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_read_byte() - SMBus read method when I2C_SMBUS_BYTE_DATA operation
+ * is only available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_byte(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx;
+
+ /* Loop over the supplied buffer receiving byte one-by-one */
+ for (idx = 0; idx < seq->bytecnt; idx++) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == seq->bytecnt - 1)
+ ccode |= CCODE_END;
+
+ /* Read data from the device */
+ sts = idt_smb_safe(read_byte, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ seq->data[idx] = (u8)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_word() - SMBus write method when I2C_SMBUS_BYTE_DATA and
+ * I2C_FUNC_SMBUS_WORD_DATA operations are available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_word(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx, evencnt;
+
+ /* Calculate the even count of data to send */
+ evencnt = seq->bytecnt - (seq->bytecnt % 2);
+
+ /* Loop over the supplied data sending two bytes at a time */
+ for (idx = 0; idx < evencnt; idx += 2) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_WORD;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == evencnt - 2)
+ ccode |= CCODE_END;
+
+ /* Send word data to the device */
+ sts = idt_smb_safe(write_word, pdev->client, ccode,
+ *(u16 *)&seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ /* If there is odd number of bytes then send just one last byte */
+ if (seq->bytecnt != evencnt) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE | CCODE_END;
+ if (idx == 0)
+ ccode |= CCODE_START;
+
+ /* Send byte data to the device */
+ sts = idt_smb_safe(write_byte, pdev->client, ccode,
+ seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_read_word() - SMBus read method when I2C_SMBUS_BYTE_DATA and
+ * I2C_FUNC_SMBUS_WORD_DATA operations are available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_word(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx, evencnt;
+
+ /* Calculate the even count of data to send */
+ evencnt = seq->bytecnt - (seq->bytecnt % 2);
+
+ /* Loop over the supplied data reading two bytes at a time */
+ for (idx = 0; idx < evencnt; idx += 2) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_WORD;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == evencnt - 2)
+ ccode |= CCODE_END;
+
+ /* Read word data from the device */
+ sts = idt_smb_safe(read_word, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ *(u16 *)&seq->data[idx] = (u16)sts;
+ }
+
+ /* If there is odd number of bytes then receive just one last byte */
+ if (seq->bytecnt != evencnt) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE | CCODE_END;
+ if (idx == 0)
+ ccode |= CCODE_START;
+
+ /* Read last data byte from the device */
+ sts = idt_smb_safe(read_byte, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ seq->data[idx] = (u8)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_block() - SMBus write method when I2C_SMBUS_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_block(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ u8 ccode;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Send block of data to the device */
+ return idt_smb_safe(write_block, pdev->client, ccode, seq->bytecnt,
+ seq->data);
+}
+
+/*
+ * idt_smb_read_block() - SMBus read method when I2C_SMBUS_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_block(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Read block of data from the device */
+ sts = idt_smb_safe(read_block, pdev->client, ccode, seq->data);
+ if (sts != seq->bytecnt)
+ return (sts < 0 ? sts : -ENODATA);
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_i2c_block() - SMBus write method when I2C_SMBUS_I2C_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ *
+ * NOTE It's usual SMBus write block operation, except the actual data length is
+ * sent as first byte of data
+ */
+static int idt_smb_write_i2c_block(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the data to send. Length byte must be added prior the data */
+ buf[0] = seq->bytecnt;
+ memcpy(&buf[1], seq->data, seq->bytecnt);
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Send length and block of data to the device */
+ return idt_smb_safe(write_i2c_block, pdev->client, ccode,
+ seq->bytecnt + 1, buf);
+}
+
+/*
+ * idt_smb_read_i2c_block() - SMBus read method when I2C_SMBUS_I2C_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ *
+ * NOTE It's usual SMBus read block operation, except the actual data length is
+ * retrieved as first byte of data
+ */
+static int idt_smb_read_i2c_block(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
+ s32 sts;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Read length and block of data from the device */
+ sts = idt_smb_safe(read_i2c_block, pdev->client, ccode,
+ seq->bytecnt + 1, buf);
+ if (sts != seq->bytecnt + 1)
+ return (sts < 0 ? sts : -ENODATA);
+ if (buf[0] != seq->bytecnt)
+ return -ENODATA;
+
+ /* Copy retrieved data to the output data buffer */
+ memcpy(seq->data, &buf[1], seq->bytecnt);
+
+ return 0;
+}
+
+/*===========================================================================
+ * EEPROM IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_eeprom_read_byte() - read just one byte from EEPROM
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @data: Data to be written to EEPROM
+ */
+static int idt_eeprom_read_byte(struct idt_89hpesx_dev *pdev, u16 memaddr,
+ u8 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_eeprom_seq eeseq;
+ struct idt_smb_seq smbseq;
+ int ret, retry;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
+ smbseq.data = (u8 *)&eeseq;
+
+ /*
+ * Sometimes EEPROM may respond with NACK if it's busy with previous
+ * operation, so we need to perform a few attempts of read cycle
+ */
+ retry = RETRY_CNT;
+ do {
+ /* Send EEPROM memory address to read data from */
+ smbseq.bytecnt = EEPROM_WRRD_CNT;
+ eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
+ eeseq.eeaddr = pdev->eeaddr;
+ eeseq.memaddr = cpu_to_le16(memaddr);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init eeprom addr 0x%02hhx",
+ memaddr);
+ break;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = EEPROM_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read eeprom data 0x%02hhx",
+ memaddr);
+ break;
+ }
+
+ /* Restart read operation if the device is busy */
+ if (retry && (eeseq.cmd & EEPROM_NAERR)) {
+ dev_dbg(dev, "EEPROM busy, retry reading after %d ms",
+ EEPROM_TOUT);
+ msleep(EEPROM_TOUT);
+ continue;
+ }
+
+ /* Check whether IDT successfully read data from EEPROM */
+ if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
+ dev_err(dev,
+ "Communication with eeprom failed, cmd 0x%hhx",
+ eeseq.cmd);
+ ret = -EREMOTEIO;
+ break;
+ }
+
+ /* Save retrieved data and exit the loop */
+ *data = eeseq.data;
+ break;
+ } while (retry--);
+
+ /* Return the status of operation */
+ return ret;
+}
+
+/*
+ * idt_eeprom_write() - EEPROM write operation
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @len: Length of data to be written
+ * @data: Data to be written to EEPROM
+ */
+static int idt_eeprom_write(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
+ const u8 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_eeprom_seq eeseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+ u16 idx;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
+ smbseq.data = (u8 *)&eeseq;
+
+ /* Send data byte-by-byte, checking if it is successfully written */
+ for (idx = 0; idx < len; idx++, memaddr++) {
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Perform write operation */
+ smbseq.bytecnt = EEPROM_WR_CNT;
+ eeseq.cmd = pdev->inieecmd | EEPROM_OP_WRITE;
+ eeseq.eeaddr = pdev->eeaddr;
+ eeseq.memaddr = cpu_to_le16(memaddr);
+ eeseq.data = data[idx];
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev,
+ "Failed to write 0x%04hx:0x%02hhx to eeprom",
+ memaddr, data[idx]);
+ goto err_mutex_unlock;
+ }
+
+ /*
+ * Check whether the data is successfully written by reading
+ * from the same EEPROM memory address.
+ */
+ eeseq.data = ~data[idx];
+ ret = idt_eeprom_read_byte(pdev, memaddr, &eeseq.data);
+ if (ret != 0)
+ goto err_mutex_unlock;
+
+ /* Check whether the read byte is the same as written one */
+ if (eeseq.data != data[idx]) {
+ dev_err(dev, "Values don't match 0x%02hhx != 0x%02hhx",
+ eeseq.data, data[idx]);
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+ if (ret != 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_eeprom_read() - EEPROM read operation
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @len: Length of data to read
+ * @buf: Buffer to read data to
+ */
+static int idt_eeprom_read(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
+ u8 *buf)
+{
+ int ret;
+ u16 idx;
+
+ /* Read data byte-by-byte, retrying if it wasn't successful */
+ for (idx = 0; idx < len; idx++, memaddr++) {
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Just read the byte to the buffer */
+ ret = idt_eeprom_read_byte(pdev, memaddr, &buf[idx]);
+
+ /* Unlock IDT SMBus device */
+ mutex_unlock(&pdev->smb_mtx);
+
+ /* Return error if read operation failed */
+ if (ret != 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*===========================================================================
+ * CSR IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_csr_write() - CSR write operation
+ * @pdev: Pointer to the driver data
+ * @csraddr: CSR address (with no two LS bits)
+ * @data: Data to be written to CSR
+ */
+static int idt_csr_write(struct idt_89hpesx_dev *pdev, u16 csraddr,
+ const u32 data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_csr_seq csrseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_CSR;
+ smbseq.data = (u8 *)&csrseq;
+
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Perform write operation */
+ smbseq.bytecnt = CSR_WR_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_WRITE;
+ csrseq.csraddr = cpu_to_le16(csraddr);
+ csrseq.data = cpu_to_le32(data);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to write 0x%04x: 0x%04x to csr",
+ CSR_REAL_ADDR(csraddr), data);
+ goto err_mutex_unlock;
+ }
+
+ /* Send CSR address to read data from */
+ smbseq.bytecnt = CSR_WRRD_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init csr address 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = CSR_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read csr 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Check whether IDT successfully retrieved CSR data */
+ if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
+ dev_err(dev, "IDT failed to perform CSR r/w");
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+
+ return ret;
+}
+
+/*
+ * idt_csr_read() - CSR read operation
+ * @pdev: Pointer to the driver data
+ * @csraddr: CSR address (with no two LS bits)
+ * @data: Data to be written to CSR
+ */
+static int idt_csr_read(struct idt_89hpesx_dev *pdev, u16 csraddr, u32 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_csr_seq csrseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_CSR;
+ smbseq.data = (u8 *)&csrseq;
+
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Send CSR register address before reading it */
+ smbseq.bytecnt = CSR_WRRD_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
+ csrseq.csraddr = cpu_to_le16(csraddr);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init csr address 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = CSR_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read csr 0x%04hx",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Check whether IDT successfully retrieved CSR data */
+ if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
+ dev_err(dev, "IDT failed to perform CSR r/w");
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Save data retrieved from IDT */
+ *data = le32_to_cpu(csrseq.data);
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+
+ return ret;
+}
+
+/*===========================================================================
+ * Sysfs/debugfs-nodes IO-operations
+ *===========================================================================
+ */
+
+/*
+ * eeprom_write() - EEPROM sysfs-node write callback
+ * @filep: Pointer to the file system node
+ * @kobj: Pointer to the kernel object related to the sysfs-node
+ * @attr: Attributes of the file
+ * @buf: Buffer to write data to
+ * @off: Offset at which data should be written to
+ * @count: Number of bytes to write
+ */
+static ssize_t eeprom_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Retrieve driver data */
+ pdev = dev_get_drvdata(kobj_to_dev(kobj));
+
+ /* Perform EEPROM write operation */
+ ret = idt_eeprom_write(pdev, (u16)off, (u16)count, (u8 *)buf);
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * eeprom_read() - EEPROM sysfs-node read callback
+ * @filep: Pointer to the file system node
+ * @kobj: Pointer to the kernel object related to the sysfs-node
+ * @attr: Attributes of the file
+ * @buf: Buffer to write data to
+ * @off: Offset at which data should be written to
+ * @count: Number of bytes to write
+ */
+static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Retrieve driver data */
+ pdev = dev_get_drvdata(kobj_to_dev(kobj));
+
+ /* Perform EEPROM read operation */
+ ret = idt_eeprom_read(pdev, (u16)off, (u16)count, (u8 *)buf);
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * idt_dbgfs_csr_write() - CSR debugfs-node write callback
+ * @filep: Pointer to the file system file descriptor
+ * @buf: Buffer to read data from
+ * @count: Size of the buffer
+ * @offp: Offset within the file
+ *
+ * It accepts either "0x<reg addr>:0x<value>" for saving register address
+ * and writing value to specified DWORD register or "0x<reg addr>" for
+ * just saving register address in order to perform next read operation.
+ *
+ * WARNING No spaces are allowed. Incoming string must be strictly formated as:
+ * "<reg addr>:<value>". Register address must be aligned within 4 bytes
+ * (one DWORD).
+ */
+static ssize_t idt_dbgfs_csr_write(struct file *filep, const char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct idt_89hpesx_dev *pdev = filep->private_data;
+ char *colon_ch, *csraddr_str, *csrval_str;
+ int ret, csraddr_len, csrval_len;
+ u32 csraddr, csrval;
+ char *buf;
+
+ /* Copy data from User-space */
+ buf = kmalloc(count + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = simple_write_to_buffer(buf, count, offp, ubuf, count);
+ if (ret < 0)
+ goto free_buf;
+ buf[count] = 0;
+
+ /* Find position of colon in the buffer */
+ colon_ch = strnchr(buf, count, ':');
+
+ /*
+ * If there is colon passed then new CSR value should be parsed as
+ * well, so allocate buffer for CSR address substring.
+ * If no colon is found, then string must have just one number with
+ * no new CSR value
+ */
+ if (colon_ch != NULL) {
+ csraddr_len = colon_ch - buf;
+ csraddr_str =
+ kmalloc(sizeof(char)*(csraddr_len + 1), GFP_KERNEL);
+ if (csraddr_str == NULL)
+ return -ENOMEM;
+ /* Copy the register address to the substring buffer */
+ strncpy(csraddr_str, buf, csraddr_len);
+ csraddr_str[csraddr_len] = '\0';
+ /* Register value must follow the colon */
+ csrval_str = colon_ch + 1;
+ csrval_len = strnlen(csrval_str, count - csraddr_len);
+ } else /* if (str_colon == NULL) */ {
+ csraddr_str = (char *)buf; /* Just to shut warning up */
+ csraddr_len = strnlen(csraddr_str, count);
+ csrval_str = NULL;
+ csrval_len = 0;
+ }
+
+ /* Convert CSR address to u32 value */
+ ret = kstrtou32(csraddr_str, 0, &csraddr);
+ if (ret != 0)
+ goto free_csraddr_str;
+
+ /* Check whether passed register address is valid */
+ if (csraddr > CSR_MAX || !IS_ALIGNED(csraddr, SZ_4)) {
+ ret = -EINVAL;
+ goto free_csraddr_str;
+ }
+
+ /* Shift register address to the right so to have u16 address */
+ csraddr >>= 2;
+
+ /* Parse new CSR value and send it to IDT, if colon has been found */
+ if (colon_ch != NULL) {
+ ret = kstrtou32(csrval_str, 0, &csrval);
+ if (ret != 0)
+ goto free_csraddr_str;
+
+ ret = idt_csr_write(pdev, (u16)csraddr, csrval);
+ if (ret != 0)
+ goto free_csraddr_str;
+ }
+
+ /* Save CSR address in the data structure for future read operations */
+ atomic_set(&pdev->csr, (int)csraddr);
+
+ /* Free memory only if colon has been found */
+free_csraddr_str:
+ if (colon_ch != NULL)
+ kfree(csraddr_str);
+
+ /* Free buffer allocated for data retrieved from User-space */
+free_buf:
+ kfree(buf);
+
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * idt_dbgfs_csr_read() - CSR debugfs-node read callback
+ * @filep: Pointer to the file system file descriptor
+ * @buf: Buffer to write data to
+ * @count: Size of the buffer
+ * @offp: Offset within the file
+ *
+ * It just prints the pair "0x<reg addr>:0x<value>" to passed buffer.
+ */
+#define CSRBUF_SIZE ((size_t)32)
+static ssize_t idt_dbgfs_csr_read(struct file *filep, char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct idt_89hpesx_dev *pdev = filep->private_data;
+ u32 csraddr, csrval;
+ char buf[CSRBUF_SIZE];
+ int ret, size;
+
+ /* Read current CSR address */
+ csraddr = atomic_read(&pdev->csr);
+
+ /* Perform CSR read operation */
+ ret = idt_csr_read(pdev, (u16)csraddr, &csrval);
+ if (ret != 0)
+ return ret;
+
+ /* Shift register address to the left so to have real address */
+ csraddr <<= 2;
+
+ /* Print the "0x<reg addr>:0x<value>" to buffer */
+ size = snprintf(buf, CSRBUF_SIZE, "0x%05x:0x%08x\n",
+ (unsigned int)csraddr, (unsigned int)csrval);
+
+ /* Copy data to User-space */
+ return simple_read_from_buffer(ubuf, count, offp, buf, size);
+}
+
+/*
+ * eeprom_attribute - EEPROM sysfs-node attributes
+ *
+ * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
+ * be read-only as well if the corresponding flag is specified in OF node.
+ */
+static BIN_ATTR_RW(eeprom, EEPROM_DEF_SIZE);
+
+/*
+ * csr_dbgfs_ops - CSR debugfs-node read/write operations
+ */
+static const struct file_operations csr_dbgfs_ops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = idt_dbgfs_csr_write,
+ .read = idt_dbgfs_csr_read
+};
+
+/*===========================================================================
+ * Driver init/deinit methods
+ *===========================================================================
+ */
+
+/*
+ * idt_set_defval() - set default device data parameters
+ * @pdev: Pointer to the driver data
+ */
+static void idt_set_defval(struct idt_89hpesx_dev *pdev)
+{
+ /* If OF info is missing then use next values */
+ pdev->eesize = EEPROM_DEF_SIZE;
+ pdev->eero = true;
+ pdev->inieecmd = 0;
+ pdev->eeaddr = EEPROM_DEF_ADDR << 1;
+}
+
+#ifdef CONFIG_OF
+/*
+ * idt_get_ofdata() - get IDT i2c-device parameters from device tree
+ * @pdev: Pointer to the driver data
+ */
+static void idt_get_ofdata(struct idt_89hpesx_dev *pdev)
+{
+ struct device_node *node = pdev->client->dev.of_node;
+ struct device *dev = &pdev->client->dev;
+ const __be32 *val;
+
+ /* Read dts node parameters */
+ if (node) {
+ /* Get EEPROM size from 'idt,eesize' */
+ val = of_get_property(node, "idt,eesize", NULL);
+ if (val != NULL) {
+ pdev->eesize = be32_to_cpup(val);
+ if (!is_power_of_2(pdev->eesize))
+ dev_warn(dev,
+ "EEPROM size %u is not power of 2",
+ pdev->eesize);
+ } else /* if (val == NULL) */ {
+ pdev->eesize = EEPROM_DEF_SIZE;
+ dev_warn(dev, "No EEPROM size, set default %u bytes",
+ pdev->eesize);
+ }
+
+ /* Get custom EEPROM address from 'idt,eeaddr' */
+ val = of_get_property(node, "idt,eeaddr", NULL);
+ if (val != NULL) {
+ pdev->inieecmd = EEPROM_USA;
+ pdev->eeaddr = be32_to_cpup(val) << 1;
+ } else /* if (val != NULL) */ {
+ pdev->inieecmd = 0;
+ pdev->eeaddr = EEPROM_DEF_ADDR << 1;
+ }
+
+ /* Check EEPROM 'read-only' flag */
+ if (of_get_property(node, "read-only", NULL))
+ pdev->eero = true;
+ else /* if (!of_get_property(node, "read-only", NULL)) */
+ pdev->eero = false;
+ } else {
+ dev_warn(dev, "No dts node, set default values");
+ idt_set_defval(pdev);
+ }
+}
+#else
+static void idt_get_ofdata(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+
+ dev_warn(dev, "OF table is unsupported, set default values");
+
+ /* Nothing we can do, just set the default values */
+ idt_set_defval(pdev);
+}
+#endif /* CONFIG_OF */
+
+/*
+ * idt_create_pdev() - create and init data structure of the driver
+ * @client: i2c client of IDT PCIe-switch device
+ */
+static struct idt_89hpesx_dev *idt_create_pdev(struct i2c_client *client)
+{
+ struct idt_89hpesx_dev *pdev;
+
+ /* Allocate memory for driver data */
+ pdev = devm_kmalloc(&client->dev, sizeof(struct idt_89hpesx_dev),
+ GFP_KERNEL);
+ if (pdev == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ /* Initialize basic fields of the data */
+ pdev->client = client;
+ i2c_set_clientdata(client, pdev);
+
+ /* Read OF nodes information */
+ idt_get_ofdata(pdev);
+
+ /* Initialize basic CSR CMD field - use full DWORD-sized r/w ops */
+ pdev->inicsrcmd = CSR_DWE;
+ atomic_set(&pdev->csr, CSR_DEF);
+
+ /* Enable Packet Error Checking if it's supported by adapter */
+ if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_PEC)) {
+ pdev->iniccode = CCODE_PEC;
+ client->flags |= I2C_CLIENT_PEC;
+ } else /* PEC is unsupported */ {
+ pdev->iniccode = 0;
+ }
+
+ dev_dbg(&client->dev, "IDT 89HPESx data created");
+
+ return pdev;
+}
+
+/*
+ * idt_free_pdev() - free data structure of the driver
+ * @pdev: Pointer to the driver data
+ */
+static void idt_free_pdev(struct idt_89hpesx_dev *pdev)
+{
+ /* Clear driver data from device private field */
+ i2c_set_clientdata(pdev->client, NULL);
+
+ /* Just free memory allocated for data */
+ devm_kfree(&pdev->client->dev, pdev);
+
+ dev_dbg(&pdev->client->dev, "IDT 89HPESx data discarded");
+}
+
+/*
+ * idt_set_smbus_ops() - set supported SMBus operations
+ * @pdev: Pointer to the driver data
+ * Return status of smbus check operations
+ */
+static int idt_set_smbus_ops(struct idt_89hpesx_dev *pdev)
+{
+ struct i2c_adapter *adapter = pdev->client->adapter;
+ struct device *dev = &pdev->client->dev;
+
+ /* Check i2c adapter read functionality */
+ if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+ pdev->smb_read = idt_smb_read_block;
+ dev_dbg(dev, "SMBus block-read op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ pdev->smb_read = idt_smb_read_i2c_block;
+ dev_dbg(dev, "SMBus i2c-block-read op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_WORD_DATA) &&
+ i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
+ pdev->smb_read = idt_smb_read_word;
+ dev_warn(dev, "Use slow word/byte SMBus read ops");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
+ pdev->smb_read = idt_smb_read_byte;
+ dev_warn(dev, "Use slow byte SMBus read op");
+ } else /* no supported smbus read operations */ {
+ dev_err(dev, "No supported SMBus read op");
+ return -EPFNOSUPPORT;
+ }
+
+ /* Check i2c adapter write functionality */
+ if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)) {
+ pdev->smb_write = idt_smb_write_block;
+ dev_dbg(dev, "SMBus block-write op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+ pdev->smb_write = idt_smb_write_i2c_block;
+ dev_dbg(dev, "SMBus i2c-block-write op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_WORD_DATA) &&
+ i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
+ pdev->smb_write = idt_smb_write_word;
+ dev_warn(dev, "Use slow word/byte SMBus write op");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
+ pdev->smb_write = idt_smb_write_byte;
+ dev_warn(dev, "Use slow byte SMBus write op");
+ } else /* no supported smbus write operations */ {
+ dev_err(dev, "No supported SMBus write op");
+ return -EPFNOSUPPORT;
+ }
+
+ /* Initialize IDT SMBus slave interface mutex */
+ mutex_init(&pdev->smb_mtx);
+
+ dev_dbg(dev, "SMBus functionality successfully checked");
+
+ return 0;
+}
+
+/*
+ * idt_check_dev() - check whether it's really IDT 89HPESx device
+ * @pdev: Pointer to the driver data
+ * Return status of i2c adapter check operation
+ */
+static int idt_check_dev(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+ u32 viddid;
+ int ret;
+
+ /* Read VID and DID directly from IDT memory space */
+ ret = idt_csr_read(pdev, IDT_VIDDID_CSR, &viddid);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read VID/DID");
+ return ret;
+ }
+
+ /* Check whether it's IDT device */
+ if ((viddid & IDT_VID_MASK) != PCI_VENDOR_ID_IDT) {
+ dev_err(dev, "Got unsupported VID/DID: 0x%08x", viddid);
+ return -ENODEV;
+ }
+
+ dev_info(dev, "Found IDT 89HPES device VID:0x%04x, DID:0x%04x",
+ (viddid & IDT_VID_MASK), (viddid >> 16));
+
+ return 0;
+}
+
+/*
+ * idt_create_sysfs_files() - create sysfs attribute files
+ * @pdev: Pointer to the driver data
+ * Return status of operation
+ */
+static int idt_create_sysfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+ int ret;
+
+ /* Allocate memory for attribute file */
+ pdev->ee_file = devm_kmalloc(dev, sizeof(*pdev->ee_file), GFP_KERNEL);
+ if (!pdev->ee_file)
+ return -ENOMEM;
+
+ /* Copy the declared EEPROM attr structure to change some of fields */
+ memcpy(pdev->ee_file, &bin_attr_eeprom, sizeof(*pdev->ee_file));
+
+ /* In case of read-only EEPROM get rid of write ability */
+ if (pdev->eero) {
+ pdev->ee_file->attr.mode &= ~0200;
+ pdev->ee_file->write = NULL;
+ }
+ /* Create EEPROM sysfs file */
+ pdev->ee_file->size = pdev->eesize;
+ ret = sysfs_create_bin_file(&dev->kobj, pdev->ee_file);
+ if (ret != 0) {
+ kfree(pdev->ee_file);
+ dev_err(dev, "Failed to create EEPROM sysfs-node");
+ return ret;
+ }
+
+ dev_dbg(dev, "Sysfs-files created");
+
+ return 0;
+}
+
+/*
+ * idt_remove_sysfs_files() - remove sysfs attribute files
+ * @pdev: Pointer to the driver data
+ */
+static void idt_remove_sysfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+
+ /* Remove EEPROM sysfs file */
+ sysfs_remove_bin_file(&dev->kobj, pdev->ee_file);
+
+ /* Free memory allocated for bin_attribute structure */
+ kfree(pdev->ee_file);
+
+ dev_dbg(dev, "Sysfs-files removed");
+}
+
+/*
+ * idt_create_dbgfs_files() - create debugfs files
+ * @pdev: Pointer to the driver data
+ * Return status of operation
+ */
+#define CSRNAME_LEN ((size_t)32)
+static int idt_create_dbgfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+ struct i2c_client *cli = pdev->client;
+ char fname[CSRNAME_LEN];
+
+ /* Initialize basic value of CSR debugfs dentries */
+ pdev->csr_dir = NULL;
+ pdev->csr_file = NULL;
+
+ /* Return failure if root directory doesn't exist */
+ if (!csr_dbgdir) {
+ dev_dbg(dev, "No Debugfs root directory");
+ return -EINVAL;
+ }
+
+ /* Create Debugfs directory for CSR file */
+ snprintf(fname, CSRNAME_LEN, "%d-%04hx", cli->adapter->nr, cli->addr);
+ pdev->csr_dir = debugfs_create_dir(fname, csr_dbgdir);
+ if (IS_ERR_OR_NULL(pdev->csr_dir)) {
+ dev_err(dev, "Failed to create CSR node directory");
+ return -EINVAL;
+ }
+
+ /* Create Debugfs file for CSR read/write operations */
+ pdev->csr_file = debugfs_create_file(cli->name, 0600,
+ pdev->csr_dir, pdev, &csr_dbgfs_ops);
+ if (IS_ERR_OR_NULL(pdev->csr_file)) {
+ dev_err(dev, "Failed to create CSR dbgfs-node");
+ debugfs_remove_recursive(pdev->csr_dir);
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "Debugfs-files created");
+
+ return 0;
+}
+
+/*
+ * idt_remove_dbgfs_files() - remove debugfs files
+ * @pdev: Pointer to the driver data
+ */
+static void idt_remove_dbgfs_files(struct idt_89hpesx_dev *pdev)
+{
+ /* Remove CSR directory and it sysfs-node */
+ debugfs_remove_recursive(pdev->csr_dir);
+
+ dev_dbg(&pdev->client->dev, "Debugfs-files removed");
+}
+
+/*
+ * idt_probe() - IDT 89HPESx driver probe() callback method
+ */
+static int idt_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Create driver data */
+ pdev = idt_create_pdev(client);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ /* Set SMBus operations */
+ ret = idt_set_smbus_ops(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Check whether it is truly IDT 89HPESx device */
+ ret = idt_check_dev(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Create sysfs files */
+ ret = idt_create_sysfs_files(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Create debugfs files */
+ (void)idt_create_dbgfs_files(pdev);
+
+ dev_dbg(&client->dev, "IDT %s device probed", id->name);
+
+ return 0;
+
+err_free_pdev:
+ idt_free_pdev(pdev);
+
+ return ret;
+}
+
+/*
+ * idt_remove() - IDT 89HPESx driver remove() callback method
+ */
+static int idt_remove(struct i2c_client *client)
+{
+ struct idt_89hpesx_dev *pdev = i2c_get_clientdata(client);
+
+ /* Remove debugfs files first */
+ idt_remove_dbgfs_files(pdev);
+
+ /* Remove sysfs files */
+ idt_remove_sysfs_files(pdev);
+
+ /* Discard driver data structure */
+ idt_free_pdev(pdev);
+
+ dev_dbg(&client->dev, "IDT 89HPESx device removed");
+
+ return 0;
+}
+
+/*
+ * idt_ids - supported IDT 89HPESx devices
+ */
+static const struct i2c_device_id idt_ids[] = {
+ { "89hpes8nt2", 0 },
+ { "89hpes12nt3", 0 },
+
+ { "89hpes24nt6ag2", 0 },
+ { "89hpes32nt8ag2", 0 },
+ { "89hpes32nt8bg2", 0 },
+ { "89hpes12nt12g2", 0 },
+ { "89hpes16nt16g2", 0 },
+ { "89hpes24nt24g2", 0 },
+ { "89hpes32nt24ag2", 0 },
+ { "89hpes32nt24bg2", 0 },
+
+ { "89hpes12n3", 0 },
+ { "89hpes12n3a", 0 },
+ { "89hpes24n3", 0 },
+ { "89hpes24n3a", 0 },
+
+ { "89hpes32h8", 0 },
+ { "89hpes32h8g2", 0 },
+ { "89hpes48h12", 0 },
+ { "89hpes48h12g2", 0 },
+ { "89hpes48h12ag2", 0 },
+ { "89hpes16h16", 0 },
+ { "89hpes22h16", 0 },
+ { "89hpes22h16g2", 0 },
+ { "89hpes34h16", 0 },
+ { "89hpes34h16g2", 0 },
+ { "89hpes64h16", 0 },
+ { "89hpes64h16g2", 0 },
+ { "89hpes64h16ag2", 0 },
+
+ /* { "89hpes3t3", 0 }, // No SMBus-slave iface */
+ { "89hpes12t3g2", 0 },
+ { "89hpes24t3g2", 0 },
+ /* { "89hpes4t4", 0 }, // No SMBus-slave iface */
+ { "89hpes16t4", 0 },
+ { "89hpes4t4g2", 0 },
+ { "89hpes10t4g2", 0 },
+ { "89hpes16t4g2", 0 },
+ { "89hpes16t4ag2", 0 },
+ { "89hpes5t5", 0 },
+ { "89hpes6t5", 0 },
+ { "89hpes8t5", 0 },
+ { "89hpes8t5a", 0 },
+ { "89hpes24t6", 0 },
+ { "89hpes6t6g2", 0 },
+ { "89hpes24t6g2", 0 },
+ { "89hpes16t7", 0 },
+ { "89hpes32t8", 0 },
+ { "89hpes32t8g2", 0 },
+ { "89hpes48t12", 0 },
+ { "89hpes48t12g2", 0 },
+ { /* END OF LIST */ }
+};
+MODULE_DEVICE_TABLE(i2c, idt_ids);
+
+/*
+ * idt_driver - IDT 89HPESx driver structure
+ */
+static struct i2c_driver idt_driver = {
+ .driver = {
+ .name = IDT_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = idt_probe,
+ .remove = idt_remove,
+ .id_table = idt_ids,
+};
+
+/*
+ * idt_init() - IDT 89HPESx driver init() callback method
+ */
+static int __init idt_init(void)
+{
+ /* Create Debugfs directory first */
+ if (debugfs_initialized())
+ csr_dbgdir = debugfs_create_dir("idt_csr", NULL);
+
+ /* Add new i2c-device driver */
+ return i2c_add_driver(&idt_driver);
+}
+module_init(idt_init);
+
+/*
+ * idt_exit() - IDT 89HPESx driver exit() callback method
+ */
+static void __exit idt_exit(void)
+{
+ /* Discard debugfs directory and all files if any */
+ debugfs_remove_recursive(csr_dbgdir);
+
+ /* Unregister i2c-device driver */
+ i2c_del_driver(&idt_driver);
+}
+module_exit(idt_exit);
--
2.6.6
^ permalink raw reply related
* [PATCH v3 0/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2016-11-29 22:27 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Serge Semin
In-Reply-To: <1480372701-30560-1-git-send-email-fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Changelog v3:
- Get rid of dev_*_idt() macros
- Replace to_pdev_kobj() macro with naked dev_get_drvdata() call
- Return naked 0 instead of SUCCESS macro
- IDT CSR debug file is moved to debugfs
- BIN_ATTR_RW is used to declare sysfs binary attribute
- Moved bindings file to a separate patch
- Need to create a specific bin_attribute structure for each device
- Perform a few read retries with delays if EEPROM is busy
Signed-off-by: Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Serge Semin (2):
eeprom: Add IDT 89HPESx EEPROM/CSR driver
eeprom: Add IDT 89HPESx driver dts-binding file
.../devicetree/bindings/misc/idt_89hpesx.txt | 41 +
drivers/misc/eeprom/Kconfig | 10 +
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/idt_89hpesx.c | 1574 ++++++++++++++++++++
4 files changed, 1626 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
create mode 100644 drivers/misc/eeprom/idt_89hpesx.c
--
2.6.6
--
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 v6 3/5] ARM: dts: sun8i-h3: add HDMI video nodes
From: Jernej Skrabec @ 2016-11-29 22:15 UTC (permalink / raw)
To: linux-sunxi
Cc: icenowy-ymACFijhrKM, airlied-cv59FeDIM0c,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20161125112213.83420594eb435b6bb1a4d164-GANU6spQydw@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]
Hi Jean-François,
Dne petek, 25. november 2016 11.22.20 UTC+1 je oseba Jean-François Moine
napisala:
>
> On Fri, 25 Nov 2016 17:41:51 +0800
> Icenowy Zheng <ice...-ymACFijhrKM@public.gmane.org <javascript:>> wrote:
>
> > After removing CLK_PLL_DE's assigned-clock, the kernel passes
> compilation.
>
> The 'pll-de' and 'de' must have a fixed rate. Otherwise, if you do not
> use the legacy u-boot, I don't know which can be the rate of the DE.
>
> > However, it cannot recognize any HDMI screen...
> >
> > (My board is Orange Pi One, and I manually added status="okay"; to
> &lcd0, &de, &hdmi)
> >
> > [ 16.507802] sun8i-de2 1000000.de-controller: bound
> 1c0c000.lcd-controller (ops de2_lcd_ops [sun8i_de2_drm])
> > [ 16.675948] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
> de2_hdmi_fini [sun8i_de2_hdmi])
> > [ 16.685120] [drm] Supports vblank timestamp caching Rev 2
> (21.10.2013).
> > [ 16.695876] [drm] No driver support for vblank timestamp query.
> > [ 16.701862] sun8i-de2 1000000.de-controller: No connectors reported
> connected with modes
> > [ 16.713061] [drm] Cannot find any crtc or sizes - going 1024x768
> > [ 16.734214] Console: switching to colour frame buffer device 128x48
> > [ 16.751022] sun8i-de2 1000000.de-controller: fb0: frame buffer
> device
>
> I put a 'pr_warn' message is case the EDID cannot be read.
> Have you this message?
>
> Anyway, there is a problem with the EDID:
> - my Orange Pi 2 (H3) randomly fails to read it. But this succeeds after
> rebooting once or twice.
>
My U-Boot driver never exhibited a problem with reading EDID on OPi2.
However,
I'm reusing code from Rockchip HDMI U-Boot driver for this (with some
Allwinner
adjustments).
> - my Banana Pi M2+ (H3) reads it correctly each time.
> - my Banana Pi M3 (A83T) can never read it.
>
> BTW, on first tries, I was forcing a CEA mode thru the kernel command
> line. This was working with the OPi2 and BPiM3, but there was no sound.
> In the last version, I use a EDID in edid_firmware for having sound
> with the BPiM3. This works fine.
> But, forcing a CEA mode is no more possible, so, when the OPi2 cannot
> read the EDID, the system switches to a VGA mode (default 1024x768)
> which is not supported. It seems that this is your case.
>
> So, in brief, if your board cannot read the EDID, put a EDID somewhere
> and its path in /sys/module/drm_kms_helper/parameters/edid_firmware.
> There will be no console, but X11 will work correctly.
>
> --
> Ken ar c'hentañ | ** Breizh ha Linux atav! **
> Jef | http://moinejf.free.fr/
Best regards,
Jernej Škrabec
--
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.
[-- Attachment #1.2: Type: text/html, Size: 4388 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND 2/2] gpio: axp209: add pinctrl support
From: Quentin Schulz @ 2016-11-29 22:13 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, Rob Herring, Mark Rutland, Chen-Yu Tsai,
Maxime Ripard, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdb8cej=AMs2kO1Vks-WYeae+CtAW2YOCc+Pz3E49mcrDQ@mail.gmail.com>
Hi Linus,
On 24/11/2016 15:17, Linus Walleij wrote:
> On Wed, Nov 23, 2016 at 3:11 PM, Quentin Schulz
> <quentin.schulz@free-electrons.com> wrote:
>
>> The GPIOs present in the AXP209 PMIC have multiple functions. They
>> typically allow a pin to be used as GPIO input or output and can also be
>> used as ADC or regulator for example.[1]
>>
>> This adds the possibility to use all functions of the GPIOs present in
>> the AXP209 PMIC thanks to pinctrl subsystem.
>>
>> [1] see registers 90H, 92H and 93H at
>> http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>
> I need Maxime's review on this patch.
>
>> .../devicetree/bindings/gpio/gpio-axp209.txt | 28 +-
>
> Also move the bindings to pinctrl/pinctrl-axp209.txt
>
>> drivers/gpio/gpio-axp209.c | 551 ++++++++++++++++++---
>
> Combined drivers should be in drivers/pinctrl/*.
>
> Make a separate patch moving the driver to
> drivers/pinctrl/pinctrl-axp209.c (remember -M to git format-patch)
> augment Kconfig and Makefile in both subsystems and make
> these patches on top of that.
>
So basically:
- first patch for adding pinctrl to the existing driver
- second patch for moving the driver and binding from gpio to pinctrl
subsystem
- third patch for both removing Kconfig entry and Makefile rule from
gpio subsystem, and adding a Kconfig entry and a Makefile rule in
pinctrl subsystem
Is that what you want?
Thanks,
Quentin
> I will deal with cross-merging the result between the GPIO
> and pin control trees.
>
> Yours,
> Linus Walleij
>
--
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eric Anholt @ 2016-11-29 22:12 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Mark Rutland, devicetree, Florian Fainelli, Russell King,
Pawel Moll, Stephen Warren, Catalin Marinas, linux-pm, Lee Jones,
Will Deacon, Rob Herring, linux-rpi-kernel, Martin Sperl,
Zhang Rui, linux-arm-kernel
In-Reply-To: <20161129013436.GA3080@localhost.localdomain>
[-- Attachment #1.1: Type: text/plain, Size: 574 bytes --]
Eduardo Valentin <edubezval@gmail.com> writes:
> Hello Eric, Martin,
>
> On Mon, Nov 28, 2016 at 12:30:38PM -0800, Eric Anholt wrote:
>> Either the device was initialized by the firmware before handing off to
>> ARM (today's firmware) or it never will be (potential future firmware).
>
> And do you have a way to check if the firmware has the initialization
> code or not? By firmware version, for example. Or even, chip version,
> maybe?
We would know if it's not present because the register would be in its
power-on reset state, which is what the code is checking for.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Maxime Ripard @ 2016-11-29 21:59 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Icenowy Zheng, Chen-Yu Tsai, Jernej Skrabec,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161128114218.14d2ed434802a20fa6b62023-GANU6spQydw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]
On Mon, Nov 28, 2016 at 11:42:18AM +0100, Jean-Francois Moine wrote:
> On Mon, 28 Nov 2016 17:59:00 +0800
> Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
>
> > As there's currently a fork of U-Boot which provides simplefb support
> > for H3, a simplefb node can be added to the device tree.
> >
> > Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> > ---
> >
> > I'm still not sure which pipeline should I use.
> >
> > And, it seems that HDMI Slow Clock is not needed?
> >
> > (seems that it's only for EDID, but simplefb won't use EDID)
>
> So, I don't see how this may work.
> How can the u-boot know the resolutions of the HDMI display device?
>
> In other words: I have a new H3 board with the last u-boot and kernel.
> I plug my (rather old or brand new) HDMI display device.
> After powering on the system, I hope to get something on the screen.
> How?
If it works like the driver for the first display engine in U-Boot, it
will use the preferred mode reported by the EDID, and will fallback to
1024x768 if it cannot access it.
Maybe it would be worth exchanging on the EDID code that has been done
for the u-boot driver too, so that it can be fixed in your driver.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v6 3/5] ARM: dts: sun8i-h3: add HDMI video nodes
From: Maxime Ripard @ 2016-11-29 21:57 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Icenowy Zheng, Dave Airlie, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
In-Reply-To: <20161125112213.83420594eb435b6bb1a4d164-GANU6spQydw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]
On Fri, Nov 25, 2016 at 11:22:13AM +0100, Jean-Francois Moine wrote:
> On Fri, 25 Nov 2016 17:41:51 +0800
> Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
>
> > After removing CLK_PLL_DE's assigned-clock, the kernel passes compilation.
>
> The 'pll-de' and 'de' must have a fixed rate. Otherwise, if you do not
> use the legacy u-boot, I don't know which can be the rate of the DE.
>
> > However, it cannot recognize any HDMI screen...
> >
> > (My board is Orange Pi One, and I manually added status="okay"; to &lcd0, &de, &hdmi)
> >
> > [ 16.507802] sun8i-de2 1000000.de-controller: bound 1c0c000.lcd-controller (ops de2_lcd_ops [sun8i_de2_drm])
> > [ 16.675948] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops de2_hdmi_fini [sun8i_de2_hdmi])
> > [ 16.685120] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> > [ 16.695876] [drm] No driver support for vblank timestamp query.
> > [ 16.701862] sun8i-de2 1000000.de-controller: No connectors reported connected with modes
> > [ 16.713061] [drm] Cannot find any crtc or sizes - going 1024x768
> > [ 16.734214] Console: switching to colour frame buffer device 128x48
> > [ 16.751022] sun8i-de2 1000000.de-controller: fb0: frame buffer device
>
> I put a 'pr_warn' message is case the EDID cannot be read.
> Have you this message?
>
> Anyway, there is a problem with the EDID:
> - my Orange Pi 2 (H3) randomly fails to read it. But this succeeds after
> rebooting once or twice.
> - my Banana Pi M2+ (H3) reads it correctly each time.
> - my Banana Pi M3 (A83T) can never read it.
>
> BTW, on first tries, I was forcing a CEA mode thru the kernel command
> line. This was working with the OPi2 and BPiM3, but there was no sound.
> In the last version, I use a EDID in edid_firmware for having sound
> with the BPiM3. This works fine.
> But, forcing a CEA mode is no more possible, so, when the OPi2 cannot
> read the EDID, the system switches to a VGA mode (default 1024x768)
> which is not supported. It seems that this is your case.
>
> So, in brief, if your board cannot read the EDID, put a EDID somewhere
> and its path in /sys/module/drm_kms_helper/parameters/edid_firmware.
> There will be no console, but X11 will work correctly.
This is one of the things that are usually very helpful to put in a
cover letter. This is obviously also a blocker for the merge of the
driver, and should be dealt with.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2016-11-29 21:45 UTC (permalink / raw)
To: Greg KH
Cc: srinivas.kandagatla, andrew, robh+dt, mark.rutland, Sergey.Semin,
linux-kernel, devicetree
In-Reply-To: <20161129212412.GA20334@kroah.com>
On Tue, Nov 29, 2016 at 10:24:12PM +0100, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Nov 30, 2016 at 12:16:25AM +0300, Serge Semin wrote:
> > On Tue, Nov 29, 2016 at 08:37:50PM +0100, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Tue, Nov 29, 2016 at 01:38:20AM +0300, Serge Semin wrote:
> > > > +struct idt_89hpesx_dev {
> > > > + u32 eesize;
> > > > + bool eero;
> > > > + u8 eeaddr;
> > > > +
> > > > + u8 inieecmd;
> > > > + u8 inicsrcmd;
> > > > + u8 iniccode;
> > > > +
> > > > + atomic_t csr;
> > > > +
> > > > + int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> > > > + int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> > > > + struct mutex smb_mtx;
> > > > +
> > > > + struct i2c_client *client;
> > > > +
> > > > + struct bin_attribute *ee_file;
> > > > + struct dentry *csr_dir;
> > > > + struct dentry *csr_file;
> > > > +};
> > > > +#define to_pdev_kobj(__kobj) \
> > > > + dev_get_drvdata(container_of(__kobj, struct device, kobj))
> > >
> > > Is it a struct device, or a kobject? This is totally confusing to me.
> > >
> > > And can't you just use kobj_to_dev()?
> > >
> >
> > I just didn't know about kobj_to_dev() inline function. Totally agree that
> > container_of() should be replaced with it.
> > What does look confusing to you? Do you mean the name "to_pdev_kobj" of the
> > macro?
>
> Yes, the macro is odd. As you are doing two different things here, just
> spell it out in the code and use kobj_to_dev() to make it easier to
> read please.
>
> > > > +/*
> > > > + * eeprom_attribute - EEPROM sysfs-node attributes
> > > > + *
> > > > + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> > > > + * be read-only as well if the corresponding flag is specified in OF node.
> > > > + */
> > > > +BIN_ATTR(eeprom, 0644, idt_sysfs_eeprom_read, idt_sysfs_eeprom_write,
> > > > + EEPROM_DEF_SIZE);
> > >
> > > static?
> > >
> > > And BIN_ATTR_RW()?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Of course it should be static. Thanks for noticing that.
> > But I intentionally utilized BIN_ATTR() instead of BIN_ATTR_RW(), because
> > the last one implies to define the read/write methods with names
> > "_name##_read"/"_name##_write", which totally get out of naming within the
> > driver source code.
>
> That's ok, use the names the macro wants you to, that's the best way,
> and it ensures that I don't have to audit your permissions are correct
> for the file.
>
> > To tell the truth macro BIN_ATTR_RW() isn't that popular in the
> > kernel.
>
> Yes, but it should be, I have patches floating around somewhere to fix
> almost all of these up.
>
> > Neither is BIN_ATTR() macro, but it suites my driver better than the
> > another one.
>
> a "raw" BIN_ATTR() shouldn't be used either, please use the _RW()
> variant.
>
> thanks,
>
> greg k-h
Agreed with all the notes. I will send patchset v3 within next hour.
Thanks,
-Sergey
^ permalink raw reply
* Re: Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Maxime Ripard @ 2016-11-29 21:43 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Icenowy Zheng, Jernej Skrabec, Jean-Francois Moine, devicetree,
linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <CAGb2v66yQ2d=12P_MYyQXTfcZnNAMcb+0i=NEab4sNcJZUyL-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]
On Mon, Nov 28, 2016 at 06:24:24PM +0800, Chen-Yu Tsai wrote:
> On Mon, Nov 28, 2016 at 6:19 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> >
> >
> > 28.11.2016, 18:07, "Chen-Yu Tsai" <wens-jdAy2FN1RRM@public.gmane.org>:
> >> On Mon, Nov 28, 2016 at 5:59 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:
> >>> As there's currently a fork of U-Boot which provides simplefb support
> >>
> >> Please add it when its finalized...
> >>
> >>> for H3, a simplefb node can be added to the device tree.
> >>>
> >>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
> >>> ---
> >>>
> >>> I'm still not sure which pipeline should I use.
> >>
> >> You are supposed to add _all_ the pipelines that are available and
> >> supported by U-boot. U-boot is then supposed to enable and update
> >> the one it set up.
> >
> > I mean the pipeline string ;-)
>
> Looks good to me. There's no separate frontend/backend in DE 2.0.
It looks good to me too.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Maxime Ripard @ 2016-11-29 21:36 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Dave Airlie, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Laurent Pinchart
In-Reply-To: <cover.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> This patchset series adds HDMI video support to the Allwinner
> sun8i SoCs which include the display engine 2 (DE2).
> The driver contains the code for the A83T and H3 SoCs, and
> some H3 boards, but it could be used/extended for other SoCs
> (A64, H2, H5) and boards (Banana PIs, Orange PIs).
Honestly, I'm getting a bit worried by the fact that you ignore
reviews.
On the important reviews that you got that are to be seen as major
issues that block the inclusion, we have:
- The fact that the HDMI driver is actually just a designware IP,
and while you should use the driver that already exists, you just
duplicated all that code.
- The fact that you ignored Rob (v6) and I (v5) comment on using OF
graph to model the connection between the display engine and the
TCON. Something that Laurent also pointed out in this version.
- The fact that you ignored that you needed an HDMI connector node
as a child of the HDMI controller. This has been reported by Rob
(v6) and yet again in this version by Laurent.
- And finally the fact that we can't have several display engine in
parallel, if needs be. This has happened in the past already on
Allwinner SoCs, so it's definitely something we should consider in
the DT bindings, since we can't break them.
Until those are fixed, I cannot see how this driver can be merged,
unfortunately.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v4] clkdev: add devm_of_clk_get()
From: Stephen Boyd @ 2016-11-29 21:26 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown, linux-clk, Linux-ARM
In-Reply-To: <87zikjw08i.wl%kuninori.morimoto.gx@renesas.com>
On 11/29, Kuninori Morimoto wrote:
>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> This patch adds it. It is implemeted in clk-devres.c to share
> devm_clk_release().
Please add an explanation of why we want this sort of API. The
example you gave for audio sound card is useful. We're not going
to remember 5 months from now why we did something, so we should
put that here instead of digging through mailing list archives.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index 8f57154..2449b25 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
> WARN_ON(ret);
> }
> EXPORT_SYMBOL(devm_clk_put);
> +
> +struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index)
Please call this devm_get_clk_from_child() instead. Also, replace
the index argument with a string called con_id. Then call
of_clk_get_by_name() instead of of_clk_get().
> +{
> + struct clk **ptr, *clk;
> +
> + ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + clk = of_clk_get(np, index);
> + if (!IS_ERR(clk)) {
> + *ptr = clk;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return clk;
> +}
> +EXPORT_SYMBOL(devm_of_clk_get);
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 123c027..7f50c5f 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -17,8 +17,9 @@
> #include <linux/notifier.h>
>
> struct device;
> -
> struct clk;
> +struct device_node;
> +struct of_phandle_args;
>
> /**
> * DOC: clk notifier callback types
> @@ -249,6 +250,21 @@ static inline void clk_unprepare(struct clk *clk)
> struct clk *devm_clk_get(struct device *dev, const char *id);
>
> /**
> + * devm_clk_get - lookup and obtain a managed reference to a clock producer.
That doesn't even match the name of the function.
> + * @dev: device for clock "consumer"
> + * @np: pointer to clock consumer node
> + * @index: clock index
> + *
> + * This function parses the clocks, and uses them to look up the
> + * struct clk from the registered list of clock providers by using
> + * @np and @index.
> + *
> + * The clock will automatically be freed when the device is unbound
> + * from the bus.
> + */
> +struct clk *devm_of_clk_get(struct device *dev, struct device_node *np, int index);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Greg KH @ 2016-11-29 21:24 UTC (permalink / raw)
To: Serge Semin
Cc: srinivas.kandagatla, andrew, robh+dt, mark.rutland, Sergey.Semin,
linux-kernel, devicetree
In-Reply-To: <20161129211625.GB9146@mobilestation>
On Wed, Nov 30, 2016 at 12:16:25AM +0300, Serge Semin wrote:
> On Tue, Nov 29, 2016 at 08:37:50PM +0100, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Nov 29, 2016 at 01:38:20AM +0300, Serge Semin wrote:
> > > +struct idt_89hpesx_dev {
> > > + u32 eesize;
> > > + bool eero;
> > > + u8 eeaddr;
> > > +
> > > + u8 inieecmd;
> > > + u8 inicsrcmd;
> > > + u8 iniccode;
> > > +
> > > + atomic_t csr;
> > > +
> > > + int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> > > + int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> > > + struct mutex smb_mtx;
> > > +
> > > + struct i2c_client *client;
> > > +
> > > + struct bin_attribute *ee_file;
> > > + struct dentry *csr_dir;
> > > + struct dentry *csr_file;
> > > +};
> > > +#define to_pdev_kobj(__kobj) \
> > > + dev_get_drvdata(container_of(__kobj, struct device, kobj))
> >
> > Is it a struct device, or a kobject? This is totally confusing to me.
> >
> > And can't you just use kobj_to_dev()?
> >
>
> I just didn't know about kobj_to_dev() inline function. Totally agree that
> container_of() should be replaced with it.
> What does look confusing to you? Do you mean the name "to_pdev_kobj" of the
> macro?
Yes, the macro is odd. As you are doing two different things here, just
spell it out in the code and use kobj_to_dev() to make it easier to
read please.
> > > +/*
> > > + * eeprom_attribute - EEPROM sysfs-node attributes
> > > + *
> > > + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> > > + * be read-only as well if the corresponding flag is specified in OF node.
> > > + */
> > > +BIN_ATTR(eeprom, 0644, idt_sysfs_eeprom_read, idt_sysfs_eeprom_write,
> > > + EEPROM_DEF_SIZE);
> >
> > static?
> >
> > And BIN_ATTR_RW()?
> >
> > thanks,
> >
> > greg k-h
>
> Of course it should be static. Thanks for noticing that.
> But I intentionally utilized BIN_ATTR() instead of BIN_ATTR_RW(), because
> the last one implies to define the read/write methods with names
> "_name##_read"/"_name##_write", which totally get out of naming within the
> driver source code.
That's ok, use the names the macro wants you to, that's the best way,
and it ensures that I don't have to audit your permissions are correct
for the file.
> To tell the truth macro BIN_ATTR_RW() isn't that popular in the
> kernel.
Yes, but it should be, I have patches floating around somewhere to fix
almost all of these up.
> Neither is BIN_ATTR() macro, but it suites my driver better than the
> another one.
a "raw" BIN_ATTR() shouldn't be used either, please use the _RW()
variant.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700
From: Wolfram Sang @ 2016-11-29 21:17 UTC (permalink / raw)
To: Romain Perier
Cc: Wolfram Sang, linux-i2c, devicetree, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala, linux-arm-kernel,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
Thomas Petazzoni, Nadav Haklai, Omri Itach, Shadi Ammouri,
Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits
In-Reply-To: <20161121133247.29889-2-romain.perier@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]
> + if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
> + i2c->fm_mask = ICR_BUSMODE_FM;
> + i2c->hs_mask = ICR_BUSMODE_HS;
> + } else {
> + i2c->fm_mask = ICR_FM;
> + i2c->hs_mask = ICR_HS;
> + }
>
> *i2c_types = (enum pxa_i2c_types)(of_id->data);
>
> @@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
> i2c->master_code = 0xe;
> i2c->rate = plat->rate;
> }
> + if (!strcmp(id->name, "armada-3700-i2c")) {
> + i2c->fm_mask = ICR_BUSMODE_FM;
> + i2c->hs_mask = ICR_BUSMODE_HS;
> + } else {
> + i2c->fm_mask = ICR_FM;
> + i2c->hs_mask = ICR_HS;
> + }
Okay, having the same code twice is not nice as well.
Sorry for missing this in the first review and going a step back, but I
think now the best solution is to have again a REGS_A3700 struct, but we
should extend it with new entries for the shifted bits. Then in the init
code, you can do something like:
i2c->fm_mask = pxa_reg_layout[i2c_type].fm_mask ?: ICR_FM;
Makes sense?
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
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