* [PATCH v6 0/5] ARM: dts: da850: tilcdc related DT changes
From: Bartosz Golaszewski @ 2016-12-12 13:05 UTC (permalink / raw)
To: Jyri Sarha, Tomi Valkeinen, David Airlie, Kevin Hilman,
Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand,
Mark Rutland, Laurent Pinchart, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Bartosz Golaszewski
This series contains the last DT changes required for LCDC support
on da850-lcdk. The first one adds the dumb-vga-dac nodes, the second
limits the maximum pixel clock rate.
v1 -> v2:
- drop patch 3/3 (already merged)
- use max-pixelclock instead of max-bandwidth for display mode limiting
v2 -> v3:
- make the commit message in patch [2/2] more detailed
- move the max-pixelclock property to da850.dtsi as the limit
affects all da850-based boards
v3 -> v4:
- remove the input port from the display node
- move the display ports node to da850-lcdk.dts
- rename the vga_bridge node to vga-bridge
- move the LCDC pins to the LCDC node (from the vga bridge node)
v4 -> v5:
- rename the display label to lcdc
- instead of using the 'dumb-vga-dac' compatible, add bindings for
ti,ths8135 and use it as the vga-bridge node compatible
v5 -> v6:
- drop the endpoint numbers for nodes with single endpoints
- split patch 2/4 from v5 into two separate patches: one adding DT
bindings and one adding actual support for ths8135
Bartosz Golaszewski (5):
ARM: dts: da850: rename the display node label
drm: bridge: add DT bindings for TI ths8135
drm: bridge: add support for TI ths8135
ARM: dts: da850-lcdk: add the vga-bridge node
ARM: dts: da850: specify the maximum pixel clock rate for tilcdc
.../bindings/display/bridge/ti,ths8135.txt | 52 +++++++++++++++++
arch/arm/boot/dts/da850-lcdk.dts | 67 ++++++++++++++++++++++
arch/arm/boot/dts/da850.dtsi | 3 +-
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
4 files changed, 122 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.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: usb:xhci: support disable usb2 LPM Remote Wakeup
From: Mathias Nyman @ 2016-12-12 13:00 UTC (permalink / raw)
To: Thang Q. Nguyen, Rob Herring
Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Mathias Nyman,
Greg Kroah-Hartman, devicetree, linux-kernel, linux-usb, Phong Vo,
Loc Ho, Vu Nguyen, patches
In-Reply-To: <CAKrQpStd4+ZZjdYyq93zq-wnqQCiQmSi5jJrYbuiZX5kVH74YA@mail.gmail.com>
On 12.12.2016 06:00, Thang Q. Nguyen wrote:
> On Sat, Dec 10, 2016 at 4:36 AM, Rob Herring <robh@kernel.org> wrote:
>> On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
>>> From: Thang Nguyen <tqnguyen@apm.com>
>>>
>>> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
>>> device or host initiated via resume signaling; device-initiated resumes
>>> can be optionally enabled/disabled by software. This patch adds support
>>> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>>>
>>> Signed-off-by: Vu Nguyen <vnguyen@apm.com>
>>> Signed-off-by: Thang Nguyen <tqnguyen@apm.com>
>>> ---
>>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>>> drivers/usb/host/xhci-plat.c | 3 +++
>>> drivers/usb/host/xhci.c | 5 ++++-
>>> drivers/usb/host/xhci.h | 1 +
>>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> index 966885c..9b4cd14 100644
>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> @@ -25,6 +25,7 @@ Required properties:
>>>
>>> Optional properties:
>>> - clocks: reference to a clock
>>> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
>>
>> Remote wakeup has been around since USB 1.0 days. Does this need to be
>> USB2 or XHCI specific?
> This is XHCI specific. Per XHCI specification 1.1, remote wakeup is
> optional for XHCI 1.0 and required for XHCI 1.1. This patch provides
> ability for software to disable RWE for USB2 in XHCI1.0 controller.
I think I understand what's going on.
USB:
The good old USB2 suspend is called L2. Device enters it after 3ms if there is no link activity.
If a device can remote wakeup (RWE) it's stated in the descriptor. RWE can be turned on
of off using standard SET/CLEAR Fature requests
The LPM L1 USB2 state again is entered with a LPM extended transaction to avoid the
3ms wait before powersaving. L1 state is exit can be done with a simialr RWE as L2 resume.
The RWE from L1 can turned on/off using a bit in the LPM extended transaction.
XHCI:
Specs say that if the device supports RWE we should enable it for LPM L1 exit as well.
This is done by setting the RWE (LPM L1) bit in PORTPMSC register. This bit only affect LPM L1 remote
wake. see 4.23.5.1.1.1
The issue might be that xhci driver never check if the device actually supports RWE, we always
set the PORTPMSC RWE (for LPM L1) bit.
How about checking something like udev->do_remote_wakeup and setting and setting the bit
based on that.
The function that you are changing, xhci_set_usb2_hardware_lpm() should only be used if
host has Hardware LPM Cabaility bit (HLC) set for that USB2 port in the
USB 2.0 xHCI Supported Protocol Capability.
Host that don't supprt LPM won't have that set. See xhci 7.2.2.1.3.2
-Mathias
^ permalink raw reply
* Re: [PATCH v7 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings
From: Lorenzo Pieralisi @ 2016-12-12 12:40 UTC (permalink / raw)
To: Juri Lelli
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
morten.rasmussen-5wv7dgnIgG8, dietmar.eggemann-5wv7dgnIgG8,
broonie-DgEjT+Ai2ygdnm+yROfE0A, Pawel Moll, Ian Campbell,
Kumar Gala, Maxime Ripard, Olof Johansson, Gregory CLEMENT,
Paul Walmsley, Linus Walleij, Chen-Yu Tsai, Thomas Petazzoni
In-Reply-To: <1473085372-2840-2-git-send-email-juri.lelli-5wv7dgnIgG8@public.gmane.org>
On Mon, Sep 05, 2016 at 03:22:45PM +0100, Juri Lelli wrote:
[...]
> +===========================================
> +5 - References
> +===========================================
> +
> +[1] ARM Linux Kernel documentation - CPUs bindings
> + Documentation/devicetree/bindings/arm/cpus.txt
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index e6782d50cbcd..c1dcf4cade2e 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -241,6 +241,14 @@ nodes to be present and contain the properties described below.
> # List of phandles to idle state nodes supported
> by this cpu [3].
>
> + - capacity-dmips-mhz
> + Usage: Optional
> + Value type: <u32>
> + Definition:
> + # u32 value representing CPU capacity [3] in
> + DMIPS/MHz, relative to highest capacity-dmips-mhz
> + in the system.
> +
> - rockchip,pmu
> Usage: optional for systems that have an "enable-method"
> property value of "rockchip,rk3066-smp"
> @@ -464,3 +472,5 @@ cpus {
> [2] arm/msm/qcom,kpss-acc.txt
> [3] ARM Linux kernel documentation - idle states bindings
> Documentation/devicetree/bindings/arm/idle-states.txt
> +[3] ARM Linux kernel documentation - cpu capacity bindings
> + Documentation/devicetree/bindings/arm/cpu-capacity.txt
Trivia: bumped into this while reviewing something else, too many
threes, you will have to fix the added reference up.
Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Sakari Ailus @ 2016-12-12 12:19 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <0f72309f-ec5e-4252-f6d7-7a7f7a9dc4c5-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
On Mon, Dec 12, 2016 at 12:15:04PM +0000, Ramiro Oliveira wrote:
> Hi Sakari
>
> On 12/12/2016 11:49 AM, Sakari Ailus wrote:
> > Hi Ramiro,
> >
> > On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
> >> Hi Sakari,
> >>
> >> Thank you for the feedback.
> >>
> >> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
> >>> Hi Ramiro,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
> >>>> Add device tree documentation.
> >>>>
> >>>> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >>>> ---
> >>>> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
> >>>> 1 file changed, 19 insertions(+)
> >>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >>>> new file mode 100644
> >>>> index 0000000..4c91b3b
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >>>> @@ -0,0 +1,19 @@
> >>>> +Omnivision OV5647 raw image sensor
> >>>> +---------------------------------
> >>>> +
> >>>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
> >>>> +and CCI (I2C compatible) control bus.
> >>>> +
> >>>> +Required properties:
> >>>> +
> >>>> +- compatible : "ovti,ov5647";
> >>>> +- reg : I2C slave address of the sensor;
> >>>> +
> >>>> +The common video interfaces bindings (see video-interfaces.txt) should be
> >>>> +used to specify link to the image data receiver. The OV5647 device
> >>>> +node should contain one 'port' child node with an 'endpoint' subnode.
> >>>> +
> >>>> +Following properties are valid for the endpoint node:
> >>>> +
> >>>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> >>>> + video-interfaces.txt. The sensor supports only two data lanes.
> >>>
> >>> Doesn't this sensor require a external clock, a reset GPIO and / or a
> >>> regulator or a few? Do you need data-lanes, unless you can change the order
> >>> or the number?
> >>
> >> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
> >> external clock but it's fixed and not controlled by SW. Should I add a property
> >> for this?
> >
> > The sensor datasheet defines a power-up and power-down sequence for the
> > device. If you don't implement these sequences in the driver on a DT based
> > system, nothing suggests that they're implemented correctly. Could it be
> > that the boot loader simply enables the regulators or another device
> > requires them to be enabled?
> >
> > I presume at least the reset GPIO should be controlled explicitly in order
> > to ensure correct function. Although hardware can be surprising: I have one
> > production system that has no reset GPIO for the sensor albeit the sensor
> > datasheet says that's part of the power up sequence.
> >
>
> Sorry for the misunderstanding. I wanted to say that, there is no SW controlled
> reset. In the board we're using to connect the sensor to our D-PHY we have a
> GPIO controller that when it receives power, it removes the sensor from reset,
> so I have no control over that.
Do you mean to say that there's a GPIO controller but there's not (yet?) a
driver for that or that the reset line is actually hard-wired to something
else?
>
> Regarding the clock, should I create a new property?
Yes. The sensor does require a clock.
>
> And also, regarding the data-lanes, AFAIK it isn't possible to change the order
> of the data and clock lanes so should I remove that property?
Sounds good to me.
>
> >>
> >>>
> >>> An example DT snippet wouldn't hurt.
> >>
> >> Sure, I can add a example snippet.
> >>
> >>>
> >>
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Peter Rosin @ 2016-12-12 12:18 UTC (permalink / raw)
To: Jonathan Cameron, Rob Herring
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Mark Rutland,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d2726499-4034-8d5d-4cff-61da86af5add-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 2016-12-10 19:21, Jonathan Cameron wrote:
> On 06/12/16 09:18, Peter Rosin wrote:
>> On 2016-12-06 00:26, Rob Herring wrote:
>>> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>>>> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>>>> ---
>>>> .../bindings/iio/multiplexer/iio-mux.txt | 40 ++++++++++++++++++++++
>>>> MAINTAINERS | 6 ++++
>>>> 2 files changed, 46 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>
>>> I'm still not convinced about this binding, but don't really have more
>>> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help
>>> either.
>>
>> Sorry about the noise, I'll try to be more careful going forward. On
>> the flip side, I haven't touched the code since v6.
>>
>> I don't see how bindings that are as flexible as the current (and
>> original) phandle link between the mux consumer and the mux controller
>> would look, and at the same time be simpler to understand. You need
>> to be able to refer to a mux controller from several mux consumers, and
>> you need to support several mux controllers in one node (the ADG792A
>> case). And, AFAICT, the complex case wasn't really the problem, it was
>> that it is overly complex to describe the simple case of one mux
>> consumer and one mux controller. But in your comment for v2 [1] you
>> said that I was working around limitations with shared GPIO pins. But
>> solving that in the GPIO subsystem would not solve all that the
>> phandle approach is solving, since you would not have support for
>> ADG792A (or other non-GPIO controlled muxes). So, I think listing
>> the gpio pins inside the mux consumer node is a non-starter, the mux
>> controller has to live in its own node with its own compatible.
>>
>> Would you be happier if I managed to marry the phandle approach with
>> the option of having the mux controller as a child node of the mux
>> consumer for the simple case?
>>
>> I added an example at the end of this message (the same as the first
>> example in v4 [2], at least in principle) for easy comparison between
>> the phandle and the controller-in-child-node approaches. I can't say
>> that I personally find the difference all that significant, and do not
>> think it is worth it. As I see it, the "simple option" would just muddy
>> the waters...
>>
>> [1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
>> [2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>> new file mode 100644
>>>> index 000000000000..8080cf790d82
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>> @@ -0,0 +1,40 @@
>>>> +IIO multiplexer bindings
>>>> +
>>>> +If a multiplexer is used to select which hardware signal is fed to
>>>> +e.g. an ADC channel, these bindings describe that situation.
>>>> +
>>>> +Required properties:
>>>> +- compatible : "iio-mux"
>>>
>>> This is a Linuxism. perhaps "adc-mux".
>>
>> No, that's not general enough, it could just as well be used to mux a
>> temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
>> "io-channel-mux" is better? That matches the io-channels property used to
>> refer to the parent channel.
> analog-mux maybe? Makes more sense out of context (though with io-channels defined on
> the next line you have plenty of context here ;)
Not that I care all that much about the name, but that doesn't really
fit if you take e.g. an IIO_INDEX channel. That sounds entirely non-analog
to me, but what do I know? Maybe that example doesn't make sense for some
reason, but I can't help but think that there will be some non-analog
channel in the future, if there isn't one already.
So, my preference is io-channel-mux, as that matches the previous dt
naming for what is muxed. But that's just my opinion, if I'm told that
it should be something else, then that's ok.
I'm more worried about other aspects, such as how to get reviewers and who
is going to take the core mux patches and what tree they are going to be
merged into etc. That is, if this series is going anywhere at all or if
someone is going to put up a road-block for some reason...
Cheers,
peda
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Ramiro Oliveira @ 2016-12-12 12:15 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <20161212114900.GS16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hi Sakari
On 12/12/2016 11:49 AM, Sakari Ailus wrote:
> Hi Ramiro,
>
> On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
>> Hi Sakari,
>>
>> Thank you for the feedback.
>>
>> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
>>> Hi Ramiro,
>>>
>>> Thank you for the patch.
>>>
>>> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
>>>> Add device tree documentation.
>>>>
>>>> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
>>>> 1 file changed, 19 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> new file mode 100644
>>>> index 0000000..4c91b3b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> @@ -0,0 +1,19 @@
>>>> +Omnivision OV5647 raw image sensor
>>>> +---------------------------------
>>>> +
>>>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
>>>> +and CCI (I2C compatible) control bus.
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible : "ovti,ov5647";
>>>> +- reg : I2C slave address of the sensor;
>>>> +
>>>> +The common video interfaces bindings (see video-interfaces.txt) should be
>>>> +used to specify link to the image data receiver. The OV5647 device
>>>> +node should contain one 'port' child node with an 'endpoint' subnode.
>>>> +
>>>> +Following properties are valid for the endpoint node:
>>>> +
>>>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>>>> + video-interfaces.txt. The sensor supports only two data lanes.
>>>
>>> Doesn't this sensor require a external clock, a reset GPIO and / or a
>>> regulator or a few? Do you need data-lanes, unless you can change the order
>>> or the number?
>>
>> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
>> external clock but it's fixed and not controlled by SW. Should I add a property
>> for this?
>
> The sensor datasheet defines a power-up and power-down sequence for the
> device. If you don't implement these sequences in the driver on a DT based
> system, nothing suggests that they're implemented correctly. Could it be
> that the boot loader simply enables the regulators or another device
> requires them to be enabled?
>
> I presume at least the reset GPIO should be controlled explicitly in order
> to ensure correct function. Although hardware can be surprising: I have one
> production system that has no reset GPIO for the sensor albeit the sensor
> datasheet says that's part of the power up sequence.
>
Sorry for the misunderstanding. I wanted to say that, there is no SW controlled
reset. In the board we're using to connect the sensor to our D-PHY we have a
GPIO controller that when it receives power, it removes the sensor from reset,
so I have no control over that.
Regarding the clock, should I create a new property?
And also, regarding the data-lanes, AFAIK it isn't possible to change the order
of the data and clock lanes so should I remove that property?
>>
>>>
>>> An example DT snippet wouldn't hurt.
>>
>> Sure, I can add a example snippet.
>>
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/2] Add support for OV5647 sensor
From: Sakari Ailus @ 2016-12-12 11:54 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <936d4f19-9a1a-0873-121e-b9471449f70d-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
On Mon, Dec 12, 2016 at 11:39:13AM +0000, Ramiro Oliveira wrote:
> Hi Sakari
>
> On 12/7/2016 11:01 PM, Sakari Ailus wrote:
> > Hi Ramiro,
> >
> > On Mon, Dec 05, 2016 at 05:36:34PM +0000, Ramiro Oliveira wrote:
> >> Add support for OV5647 sensor.
> >>
> >> Modes supported:
> >> - 640x480 RAW 8
> >>
> >> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> ---
> >> MAINTAINERS | 7 +
> >> drivers/media/i2c/Kconfig | 12 +
> >> drivers/media/i2c/Makefile | 1 +
> >> drivers/media/i2c/ov5647.c | 866 +++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 886 insertions(+)
> >> create mode 100644 drivers/media/i2c/ov5647.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 52cc077..72e828a 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -8923,6 +8923,13 @@ M: Harald Welte <laforge-TgoAw6mPHtdg9hUCZPvPmw@public.gmane.org>
> >> S: Maintained
> >> F: drivers/char/pcmcia/cm4040_cs.*
> >>
> >> +OMNIVISION OV5647 SENSOR DRIVER
> >> +M: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> +L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> +T: git git://linuxtv.org/media_tree.git
> >> +S: Maintained
> >> +F: drivers/media/i2c/ov5647.c
> >> +
> >> OMNIVISION OV7670 SENSOR DRIVER
> >> M: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> >> L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> >> index b31fa6f..c1b78e5 100644
> >> --- a/drivers/media/i2c/Kconfig
> >> +++ b/drivers/media/i2c/Kconfig
> >> @@ -531,6 +531,18 @@ config VIDEO_OV2659
> >> To compile this driver as a module, choose M here: the
> >> module will be called ov2659.
> >>
> >> +config VIDEO_OV5647
> >> + tristate "OmniVision OV5647 sensor support"
> >> + depends on OF
> >
> > How does this driver depend on OF, other than matching the compatible
> > string?
> >
>
> It doesn't, should I proceed diferently?
You should drop the dependency if you don't need it. But I bet you will
actually need it.
>
> >> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> >> + depends on MEDIA_CAMERA_SUPPORT
> >> + ---help---
> >> + This is a Video4Linux2 sensor-level driver for the OmniVision
> >> + OV5647 camera.
> >> +
> >> + To compile this driver as a module, choose M here: the
> >> + module will be called ov5647.
> >> +
> >> config VIDEO_OV7640
> >> tristate "OmniVision OV7640 sensor support"
> >> depends on I2C && VIDEO_V4L2
> >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> >> index 92773b2..0d9014c 100644
> >> --- a/drivers/media/i2c/Makefile
> >> +++ b/drivers/media/i2c/Makefile
> >> @@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
> >> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> >> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> >> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> >> +obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
> >> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> >> new file mode 100644
> >> index 0000000..2aae806
> >> --- /dev/null
> >> +++ b/drivers/media/i2c/ov5647.c
> >> @@ -0,0 +1,866 @@
> >> +/*
> >> + * A V4L2 driver for OmniVision OV5647 cameras.
> >> + *
> >> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
> >> + * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> + *
> >> + * Based on Omnivision OV7670 Camera Driver
> >> + * Copyright (C) 2006-7 Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> >> + *
> >> + * Copyright (C) 2016, Synopsys, Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation version 2.
> >> + *
> >> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> >> + * kind, whether express or implied; without even the implied warranty
> >> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +#include <linux/init.h>
> >> +#include <linux/module.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/videodev2.h>
> >> +#include <media/v4l2-device.h>
> >> +#include <media/v4l2-ctrls.h>
> >> +#include <media/v4l2-mediabus.h>
> >> +#include <media/v4l2-image-sizes.h>
> >> +#include <media/v4l2-of.h>
> >> +#include <linux/io.h>
> >
> > Alphabetical order, please.
> >
> >> +
> >> +#define SENSOR_NAME "ov5647"
> >> +
> >> +#define OV5647_SW_RESET 0x1003
> >> +#define OV5647_REG_CHIPID_H 0x300A
> >> +#define OV5647_REG_CHIPID_L 0x300B
> >> +
> >> +#define REG_TERM 0xfffe
> >> +#define VAL_TERM 0xfe
> >> +#define REG_DLY 0xffff
> >> +
> >> +#define OV5647_ROW_START 0x01
> >> +#define OV5647_ROW_START_MIN 0
> >> +#define OV5647_ROW_START_MAX 2004
> >> +#define OV5647_ROW_START_DEF 54
> >> +
> >> +#define OV5647_COLUMN_START 0x02
> >> +#define OV5647_COLUMN_START_MIN 0
> >> +#define OV5647_COLUMN_START_MAX 2750
> >> +#define OV5647_COLUMN_START_DEF 16
> >> +
> >> +#define OV5647_WINDOW_HEIGHT 0x03
> >> +#define OV5647_WINDOW_HEIGHT_MIN 2
> >> +#define OV5647_WINDOW_HEIGHT_MAX 2006
> >> +#define OV5647_WINDOW_HEIGHT_DEF 1944
> >> +
> >> +#define OV5647_WINDOW_WIDTH 0x04
> >> +#define OV5647_WINDOW_WIDTH_MIN 2
> >> +#define OV5647_WINDOW_WIDTH_MAX 2752
> >> +#define OV5647_WINDOW_WIDTH_DEF 2592
> >> +
> >> +struct regval_list {
> >> + u16 addr;
> >> + u8 data;
> >> +};
> >> +
> >> +struct cfg_array {
> >> + struct regval_list *regs;
> >> + int size;
> >> +};
> >> +
> >> +struct sensor_win_size {
> >> + int width;
> >> + int height;
> >> + unsigned int hoffset;
> >> + unsigned int voffset;
> >> + unsigned int hts;
> >> + unsigned int vts;
> >> + unsigned int pclk;
> >> + unsigned int mipi_bps;
> >> + unsigned int fps_fixed;
> >> + unsigned int bin_factor;
> >> + unsigned int intg_min;
> >> + unsigned int intg_max;
> >> + void *regs;
> >> + int regs_size;
> >> + int (*set_size)(struct v4l2_subdev *sd);
> >> +};
> >> +
> >> +
> >> +struct ov5647 {
> >> + struct device *dev;
> >> + struct v4l2_subdev sd;
> >> + struct media_pad pad;
> >> + struct mutex lock;
> >> + struct v4l2_mbus_framefmt format;
> >> + struct sensor_format_struct *fmt;
> >> + unsigned int width;
> >> + unsigned int height;
> >> + unsigned int capture_mode;
> >> + int hue;
> >
> > At least capture_mode and hue are unused. Please remove unused fields.
> >
> >> + struct v4l2_fract tpf;
> >> + struct sensor_win_size *current_wins;
> >> +};
> >> +
> >> +static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
> >> +{
> >> + return container_of(sd, struct ov5647, sd);
> >> +}
> >> +
> >> +static struct regval_list sensor_oe_disable_regs[] = {
> >> + {0x3000, 0x00},
> >> + {0x3001, 0x00},
> >> + {0x3002, 0x00},
> >> +};
> >> +
> >> +static struct regval_list sensor_oe_enable_regs[] = {
> >> + {0x3000, 0x0f},
> >> + {0x3001, 0xff},
> >> + {0x3002, 0xe4},
> >> +};
> >> +
> >> +static struct regval_list ov5647_640x480[] = {
> >
> > Does this list expect a certain external clock frequency? If it does, should
> > you check that the actual frequency matches with the expectation?
> >
>
> Yes. But like I said in the DT patch the external clock has a fixed frequency. I
> can add a check if you thinks it's better.
>
> >> + {0x0100, 0x00},
> >> + {0x0103, 0x01},
> >> + {0x3034, 0x08},
> >> + {0x3035, 0x21},
> >> + {0x3036, 0x46},
> >> + {0x303c, 0x11},
> >> + {0x3106, 0xf5},
> >> + {0x3821, 0x07},
> >> + {0x3820, 0x41},
> >> + {0x3827, 0xec},
> >> + {0x370c, 0x0f},
> >> + {0x3612, 0x59},
> >> + {0x3618, 0x00},
> >> + {0x5000, 0x06},
> >> + {0x5001, 0x01},
> >> + {0x5002, 0x41},
> >> + {0x5003, 0x08},
> >> + {0x5a00, 0x08},
> >> + {0x3000, 0x00},
> >> + {0x3001, 0x00},
> >> + {0x3002, 0x00},
> >> + {0x3016, 0x08},
> >> + {0x3017, 0xe0},
> >> + {0x3018, 0x44},
> >> + {0x301c, 0xf8},
> >> + {0x301d, 0xf0},
> >> + {0x3a18, 0x00},
> >> + {0x3a19, 0xf8},
> >> + {0x3c01, 0x80},
> >> + {0x3b07, 0x0c},
> >> + {0x380c, 0x07},
> >> + {0x380d, 0x68},
> >> + {0x380e, 0x03},
> >> + {0x380f, 0xd8},
> >> + {0x3814, 0x31},
> >> + {0x3815, 0x31},
> >> + {0x3708, 0x64},
> >> + {0x3709, 0x52},
> >> + {0x3808, 0x02},
> >> + {0x3809, 0x80},
> >> + {0x380a, 0x01},
> >> + {0x380b, 0xE0},
> >> + {0x3801, 0x00},
> >> + {0x3802, 0x00},
> >> + {0x3803, 0x00},
> >> + {0x3804, 0x0a},
> >> + {0x3805, 0x3f},
> >> + {0x3806, 0x07},
> >> + {0x3807, 0xa1},
> >> + {0x3811, 0x08},
> >> + {0x3813, 0x02},
> >> + {0x3630, 0x2e},
> >> + {0x3632, 0xe2},
> >> + {0x3633, 0x23},
> >> + {0x3634, 0x44},
> >> + {0x3636, 0x06},
> >> + {0x3620, 0x64},
> >> + {0x3621, 0xe0},
> >> + {0x3600, 0x37},
> >> + {0x3704, 0xa0},
> >> + {0x3703, 0x5a},
> >> + {0x3715, 0x78},
> >> + {0x3717, 0x01},
> >> + {0x3731, 0x02},
> >> + {0x370b, 0x60},
> >> + {0x3705, 0x1a},
> >> + {0x3f05, 0x02},
> >> + {0x3f06, 0x10},
> >> + {0x3f01, 0x0a},
> >> + {0x3a08, 0x01},
> >> + {0x3a09, 0x27},
> >> + {0x3a0a, 0x00},
> >> + {0x3a0b, 0xf6},
> >> + {0x3a0d, 0x04},
> >> + {0x3a0e, 0x03},
> >> + {0x3a0f, 0x58},
> >> + {0x3a10, 0x50},
> >> + {0x3a1b, 0x58},
> >> + {0x3a1e, 0x50},
> >> + {0x3a11, 0x60},
> >> + {0x3a1f, 0x28},
> >> + {0x4001, 0x02},
> >> + {0x4004, 0x02},
> >> + {0x4000, 0x09},
> >> + {0x4837, 0x24},
> >> + {0x4050, 0x6e},
> >> + {0x4051, 0x8f},
> >> + {0x0100, 0x01},
> >> +};
> >> +
> >> +struct sensor_format_struct;
> >> +
> >> +/**
> >> + * @short I2C Write operation
> >> + * @param[in] i2c_client I2C client
> >> + * @param[in] reg register address
> >> + * @param[in] val value to write
> >> + * @return Error code
> >> + */
> >> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> >> +{
> >> + int ret;
> >> + unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = i2c_master_send(client, data, 3);
> >> + if (ret != 3) {
> >> + dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short I2C Read operation
> >> + * @param[in] i2c_client I2C client
> >> + * @param[in] reg register address
> >> + * @param[out] val value read
> >> + * @return Error code
> >> + */
> >> +static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
> >> +{
> >> + int ret;
> >> + unsigned char data_w[2] = { reg >> 8, reg & 0xff };
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> +
> >> + ret = i2c_master_send(client, data_w, 2);
> >> +
> >> + if (ret < 2) {
> >> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> +
> >> +
> >> + ret = i2c_master_recv(client, val, 1);
> >> +
> >> + if (ret < 1) {
> >> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static int ov5647_write_array(struct v4l2_subdev *sd,
> >> + struct regval_list *regs, int array_size)
> >> +{
> >> + int i = 0;
> >> + int ret = 0;
> >> +
> >> + if (!regs)
> >> + return -EINVAL;
> >> +
> >> + while (i < array_size) {
> >> + ret = ov5647_write(sd, regs->addr, regs->data);
> >> + if (ret < 0)
> >> + return ret;
> >> + i++;
> >> + regs++;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
> >> +{
> >> + u8 channel_id;
> >> +
> >> + ov5647_read(sd, 0x4814, &channel_id);
> >> + channel_id &= ~(3 << 6);
> >> + ov5647_write(sd, 0x4814, channel_id | (channel << 6));
> >> +}
> >> +
> >> +void ov5647_stream_on(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ov5647_write(sd, 0x4202, 0x00);
> >> + dev_dbg(&client->dev, "Stream on");
> >> + ov5647_write(sd, 0x300D, 0x00);
> >> +}
> >> +
> >> +void ov5647_stream_off(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ov5647_write(sd, 0x4202, 0x0f);
> >> + dev_dbg(&client->dev, "Stream off");
> >> + ov5647_write(sd, 0x300D, 0x01);
> >> +}
> >> +
> >> +/****************************************************************************/
> >> +
> >> +/**
> >> + * @short Set SW standby
> >> + * @param[in] sd v4l2 sd
> >> + * @param[in] stanby standby mode status (on or off)
> >> + * @return Error code
> >> + */
> >> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
> >> +{
> >> + int ret;
> >> + unsigned char rdval;
> >> +
> >> + ret = ov5647_read(sd, 0x0100, &rdval);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + if (standby)
> >> + rdval &= 0xfe;
> >> + else
> >> + rdval |= 0x01;
> >> +
> >> + ret = ov5647_write(sd, 0x0100, rdval);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Store information about the video data format.
> >> + */
> >> +static struct sensor_format_struct {
> >> + u8 *desc;
> >> + u32 mbus_code;
> >> + struct regval_list *regs;
> >> + int regs_size;
> >> + int bpp;
> >
> > At least desc and bpp are unused.
> >
> >> +} sensor_formats[] = {
> >> + {
> >> + .desc = "Raw RGB Bayer",
> >> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> >> + .regs = ov5647_640x480,
> >> + .regs_size = ARRAY_SIZE(ov5647_640x480),
> >> + .bpp = 1
> >> + },
> >> +};
> >> +#define N_FMTS ARRAY_SIZE(sensor_formats)
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Initialize sensor
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] val not used
> >> + * @return Error code
> >> + */
> >> +static int __sensor_init(struct v4l2_subdev *sd)
> >> +{
> >> + int ret;
> >> + u8 resetval;
> >> + u8 rdval;
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + dev_dbg(&client->dev, "sensor init\n");
> >> +
> >> + ret = ov5647_read(sd, 0x0100, &rdval);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + ov5647_write(sd, 0x4800, 0x25);
> >> + ov5647_stream_off(sd);
> >> +
> >> + ret = ov5647_write_array(sd, ov5647_640x480,
> >> + ARRAY_SIZE(ov5647_640x480));
> >> + if (ret < 0) {
> >> + dev_err(&client->dev, "write sensor_default_regs error\n");
> >> + return ret;
> >> + }
> >> +
> >> + ov5647_set_virtual_channel(sd, 0);
> >> +
> >> + ov5647_read(sd, 0x0100, &resetval);
> >> + if (!(resetval & 0x01)) {
> >> + dev_err(&client->dev, "Device was in SW standby");
> >> + ov5647_write(sd, 0x0100, 0x01);
> >> + }
> >> +
> >> + ov5647_write(sd, 0x4800, 0x04);
> >> + ov5647_stream_on(sd);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Control sensor power state
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] on Sensor power
> >> + * @return Error code
> >> + */
> >> +static int sensor_power(struct v4l2_subdev *sd, int on)
> >> +{
> >> + int ret;
> >> + struct ov5647 *ov5647 = to_state(sd);
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = 0;
> >> + mutex_lock(&ov5647->lock);
> >> +
> >> + if (on) {
> >> + dev_dbg(&client->dev, "OV5647 power on\n");
> >> +
> >> + ret = ov5647_write_array(sd, sensor_oe_enable_regs,
> >> + ARRAY_SIZE(sensor_oe_enable_regs));
> >> +
> >> + ret = __sensor_init(sd);
> >> +
> >> + if (ret < 0)
> >> + dev_err(&client->dev,
> >> + "Camera not available, check Power\n");
> >> + } else {
> >> + dev_dbg(&client->dev, "OV5647 power off\n");
> >> +
> >> + dev_dbg(&client->dev, "disable oe\n");
> >> + ret = ov5647_write_array(sd, sensor_oe_disable_regs,
> >> + ARRAY_SIZE(sensor_oe_disable_regs));
> >> +
> >> + if (ret < 0)
> >> + dev_dbg(&client->dev, "disable oe failed\n");
> >> +
> >> + ret = set_sw_standby(sd, true);
> >> +
> >> + if (ret < 0)
> >> + dev_dbg(&client->dev, "soft stby failed\n");
> >> +
> >> + }
> >> +
> >> + mutex_unlock(&ov5647->lock);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> +/**
> >> + * @short Get register value
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] reg register struct
> >> + * @return Error code
> >> + */
> >> +static int sensor_get_register(struct v4l2_subdev *sd,
> >> + struct v4l2_dbg_register *reg)
> >> +{
> >> + unsigned char val = 0;
> >> + int ret;
> >> +
> >> + ret = ov5647_read(sd, reg->reg & 0xff, &val);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + reg->val = val;
> >> + reg->size = 1;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Set register value
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] reg register struct
> >> + * @return Error code
> >> + */
> >> +static int sensor_set_register(struct v4l2_subdev *sd,
> >> + const struct v4l2_dbg_register *reg)
> >> +{
> >> + return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> >> +}
> >> +#endif
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Subdev core operations registration
> >> + */
> >> +static const struct v4l2_subdev_core_ops sensor_core_ops = {
> >> + .s_power = sensor_power,
> >
> > The s_power() op will be called by the bridge (ISP) driver as well. You
> > should expect that it may be enabled more than once before being disabled
> > equal number of times.
> >
>
> Should I add an IF check to verify if the sensor is powered on before running
> the power on/off routine.
You need a counter. One way to do that is in the driver itself, or you can
do what I recently did in the smiapp driver, using runtime PM so you don't
explicitly need to manage it:
<URL:https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=smiapp-pm&id=c29df33f9ec94226eab8ee92d8c66ab83c76659a>
Or, before the runtime PM conversion:
<URL:https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=smiapp-pm&id=c8d2bc9bc39ebea8437fd974fdbc21847bb897a3>
The use of runtime PM in sub-device drivers is new and I'm not entirely
happy how it's implemented in the smiapp driver right now, hence it might be
better not to use it (yet). Up to you.
>
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> + .g_register = sensor_get_register,
> >> + .s_register = sensor_set_register,
> >> +#endif
> >> +};
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +
> >> +
> >> +/**
> >> + * @short Enumerate available image formats
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] index index
> >> + * @param[in] code MBUS Pixel code
> >> + * @return Error code
> >> + */
> >> +static int sensor_enum_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_mbus_code_enum *code)
> >> +{
> >> + if (code->pad || code->index >= N_FMTS)
> >> + return -EINVAL;
> >> +
> >> + code->code = sensor_formats[code->index].mbus_code;
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Try frame format internal function
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fmt frame format
> >> + * @return Error code
> >> + */
> >> +static int sensor_try_fmt_internal(struct v4l2_subdev *sd,
> >> + struct v4l2_mbus_framefmt *fmt, struct sensor_format_struct **ret_fmt,
> >> + struct sensor_win_size **ret_wsize)
> >> +{
> >> + int index;
> >> +
> >> + for (index = 0; index < N_FMTS; index++)
> >> + if (sensor_formats[index].mbus_code == fmt->code)
> >> + break;
> >> +
> >> + if (index >= N_FMTS)
> >> + return -EINVAL;
> >> +
> >> + if (ret_fmt != NULL)
> >> + *ret_fmt = sensor_formats + index;
> >> +
> >> + fmt->field = V4L2_FIELD_NONE;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Set frame format
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fmt frame format
> >> + * @return Error code
> >> + */
> >> +static int sensor_s_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_format *fmt)
> >> +{
> >> + int ret;
> >> + struct sensor_format_struct *sensor_fmt;
> >> + struct sensor_win_size *wsize;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + ov5647_write_array(sd, sensor_oe_disable_regs,
> >> + ARRAY_SIZE(sensor_oe_disable_regs));
> >
> > Should you check the error code here?
> >
> >> +
> >> + ret = sensor_try_fmt_internal(sd, &fmt->format,
> >> + &sensor_fmt, &wsize);
> >
> > Do you set wsize somewhere?
> >
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ov5647_write_array(sd, sensor_fmt->regs, sensor_fmt->regs_size);
> >
> > And here.
> >
> >> +
> >> + ret = 0;
> >
> > ret was already zero.
> >
> >> +
> >> + if (wsize->regs)
> >> + ov5647_write_array(sd, wsize->regs, wsize->regs_size);
> >> +
> >> + if (wsize->set_size)
> >> + wsize->set_size(sd);
> >> +
> >> + info->fmt = sensor_fmt;
> >> + info->width = wsize->width;
> >> + info->height = wsize->height;
> >> +
> >> + ov5647_write_array(sd, sensor_oe_enable_regs,
> >> + ARRAY_SIZE(sensor_oe_enable_regs));
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Set stream parameters
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] parms stream parameters
> >> + * @return Error code
> >> + */
> >> +static int sensor_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >> +{
> >> + struct v4l2_captureparm *cp = &parms->parm.capture;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> + return -EINVAL;
> >> +
> >> + if (info->tpf.numerator == 0)
> >> + return -EINVAL;
> >> +
> >> + info->capture_mode = cp->capturemode;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Get stream parameters
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] parms stream parameters
> >> + * @return Error code
> >> + */
> >> +static int sensor_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >> +{
> >> + struct v4l2_captureparm *cp = &parms->parm.capture;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> + return -EINVAL;
> >> +
> >> + memset(cp, 0, sizeof(struct v4l2_captureparm));
> >> + cp->capability = V4L2_CAP_TIMEPERFRAME;
> >> + cp->capturemode = info->capture_mode;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Subdev video operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_video_ops sensor_video_ops = {
> >> + .s_parm = sensor_s_parm,
> >> + .g_parm = sensor_g_parm,
> >
> > Please use the g/s_frame_interval() instead. That's what sub-device drivers
> > generally use for the purpose.
> >
> >> +};
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Subdev operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_ops subdev_ops = {
> >> + .core = &sensor_core_ops,
> >> + .video = &sensor_video_ops,
> >> +};
> >> +
> >> +/* -----------------------------------------------------------------------------
> >> + * V4L2 subdev internal operations
> >> + */
> >> +
> >> +/**
> >> + * @short Detect camera version and model
> >> + * @param[in] sd v4l2 subdev
> >> + * @return Error code
> >> + */
> >> +int ov5647_detect(struct v4l2_subdev *sd)
> >> +{
> >> + unsigned char v;
> >> + int ret;
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
> >> + if (ret < 0)
> >> + return ret;
> >> + ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
> >> + if (ret < 0)
> >> + return ret;
> >> + if (v != 0x56) {
> >> + dev_err(&client->dev, "Wrong model version detected");
> >> + return -ENODEV;
> >> + }
> >> + ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
> >> + if (ret < 0)
> >> + return ret;
> >> + if (v != 0x47) {
> >> + dev_err(&client->dev, "Wrong model version detected");
> >> + return -ENODEV;
> >> + }
> >> +
> >> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Detect if camera is registered
> >> + * @param[in] sd v4l2 subdev
> >> + * @return Error code
> >> + */
> >> +static int ov5647_registered(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + dev_info(&client->dev, "OV5647 detected at address 0x%02x\n",
> >> + client->addr);
> >
> > I might omit this. If there's a need to debug this then the information
> > should be printed by the framework instead using debug level messages.
> >
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Open device
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fh v4l2 file handler
> >> + * @return Error code
> >> + */
> >> +static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> >> +{
> >> + struct v4l2_mbus_framefmt *format =
> >> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
> >> + struct v4l2_rect *crop =
> >> + v4l2_subdev_get_try_crop(sd, fh->pad, 0);
> >> +
> >> + crop->left = OV5647_COLUMN_START_DEF;
> >> + crop->top = OV5647_ROW_START_DEF;
> >> + crop->width = OV5647_WINDOW_WIDTH_DEF;
> >> + crop->height = OV5647_WINDOW_HEIGHT_DEF;
> >> +
> >> + format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
> >> +
> >> + format->width = OV5647_WINDOW_WIDTH_DEF;
> >> + format->height = OV5647_WINDOW_HEIGHT_DEF;
> >> + format->field = V4L2_FIELD_NONE;
> >> + format->colorspace = V4L2_COLORSPACE_SRGB;
> >> +
> >> + return sensor_power(sd, true);
> >> +}
> >> +
> >> +/**
> >> + * @short Open device
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fh v4l2 file handler
> >> + * @return Error code
> >> + */
> >> +static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> >> +{
> >> + return sensor_power(sd, false);
> >> +}
> >> +
> >> +/**
> >> + * @short Subdev internal operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
> >> + .registered = ov5647_registered,
> >> + .open = ov5647_open,
> >> + .close = ov5647_close,
> >> +};
> >> +
> >> +/**
> >> + * @short Initialization routine - Entry point of the driver
> >> + * @param[in] client pointer to the i2c client structure
> >> + * @param[in] id pointer to the i2c device id structure
> >> + * @return 0 on success and a negative number on failure
> >> + */
> >> +static int ov5647_probe(struct i2c_client *client,
> >> + const struct i2c_device_id *id)
> >> +{
> >> + struct device *dev = &client->dev;
> >> + struct ov5647 *sensor;
> >> + int ret = 0;
> >
> > No need to initialise ret here.
> >
> >> + struct v4l2_subdev *sd;
> >> +
> >> + dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
> >> +
> >> + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> >> + if (sensor == NULL)
> >> + return -ENOMEM;
> >> +
> >> + mutex_init(&sensor->lock);
> >> + sensor->dev = dev;
> >> +
> >> + sd = &sensor->sd;
> >> + v4l2_i2c_subdev_init(sd, client, &subdev_ops);
> >> + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >> +
> >> + sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
> >> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> >> + ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
> >> + if (ret < 0)
> >> + goto mutex_remove;
> >> +
> >> + ret = ov5647_detect(sd);
> >> + if (ret < 0)
> >> + goto error;
> >> +
> >> + ret = v4l2_async_register_subdev(sd);
> >> + if (ret < 0)
> >> + goto error;
> >> +
> >> + return 0;
> >> +error:
> >> + media_entity_cleanup(&sd->entity);
> >> +mutex_remove:
> >> + mutex_destroy(&sensor->lock);
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Exit routine - Exit point of the driver
> >> + * @param[in] client pointer to the i2c client structure
> >> + * @return 0 on success and a negative number on failure
> >> + */
> >> +static int ov5647_remove(struct i2c_client *client)
> >> +{
> >> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> >> + struct ov5647 *ov5647 = to_state(sd);
> >> +
> >> + v4l2_async_unregister_subdev(&ov5647->sd);
> >> + media_entity_cleanup(&ov5647->sd.entity);
> >> + v4l2_device_unregister_subdev(sd);
> >> + mutex_destroy(&ov5647->lock);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct i2c_device_id ov5647_id[] = {
> >> + { "ov5647", 0 },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, ov5647_id);
> >> +
> >> +#if IS_ENABLED(CONFIG_OF)
> >> +static const struct of_device_id ov5647_of_match[] = {
> >> + { .compatible = "ovti,ov5647" },
> >> + { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, ov5647_of_match);
> >> +#endif
> >> +
> >> +/**
> >> + * @short i2c driver structure
> >> + */
> >> +static struct i2c_driver ov5647_driver = {
> >> + .driver = {
> >> + .of_match_table = of_match_ptr(ov5647_of_match),
> >> + .owner = THIS_MODULE,
> >> + .name = "ov5647",
> >> + },
> >> + .probe = ov5647_probe,
> >> + .remove = ov5647_remove,
> >> + .id_table = ov5647_id,
> >> +};
> >> +
> >> +module_i2c_driver(ov5647_driver);
> >> +
> >> +MODULE_AUTHOR("Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>");
> >> +MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
> >> +MODULE_LICENSE("GPL v2");
> >
--
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] PCI: rockchip: Add quirk to disable RC's ASPM L0s
From: Shawn Lin @ 2016-12-12 11:51 UTC (permalink / raw)
To: Bjorn Helgaas, Rob Herring
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wenrui Li,
Brian Norris, Jeffy Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
Shawn Lin
Rockchip's RC outputs 100MHz reference clock but there are
two methods for PHY to generate it.
(1)One of them is to use system PLL to generate 100MHz clock and
the PHY will relock it and filter signal noise then outputs the
reference clock.
(2)Another way is to share Soc's 24MHZ crystal oscillator with
PHY and force PHY's DLL to generate 100MHz internally.
When using case(2), the exit from L0s doesn't work fine occasionally
due to the broken design of RC receiver's logical circuit. So even if
we use extended-synch, it still fails for PHY to relock the bits from
FTS sometimes. This will hang the system.
Maybe we could argue that why not use case(1) to avoid it? The reason
is that as we could see the reference clock is derived from system PLL
and the path from it to PHY isn't so clean which means there are some
noise introduced by power-domain and other buses can't be filterd out
by PHY and we could see noise from the frequency spectrum by
oscilloscope. This makes the TX compatibility test a little difficult
to pass the spec. So case(1) and case(2) are both used indeed now. If
using case(2), we should disable RC's L0s support, and that is why we
need this property to indicate this quirk.
Also after checking quirk.c, I noticed there is already a quirk for
disabling L0s unconditionally, quirk_disable_aspm_l0s. But obviously we
shouldn't do that as mentioned above that case(1) could still works fine
with L0s.
Reported-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Changes in v2:
- drop the quirk prefix
Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 2 ++
drivers/pci/host/pcie-rockchip.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index 71aeda1..1453a73 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -43,6 +43,8 @@ Required properties:
- interrupt-map-mask and interrupt-map: standard PCI properties
Optional Property:
+- aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
+ using 24MHz OSC for RC's PHY.
- ep-gpios: contain the entry for pre-reset gpio
- num-lanes: number of lanes to use
- vpcie3v3-supply: The phandle to the 3.3v regulator to use for PCIe.
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index f2dca7b..35988fc 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -140,6 +140,8 @@
#define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
#define PCIE_RC_CONFIG_DCR_CSPL_LIMIT 0xff
#define PCIE_RC_CONFIG_DCR_CPLS_SHIFT 26
+#define PCIE_RC_CONFIG_LINK_CAP (PCIE_RC_CONFIG_BASE + 0xcc)
+#define PCIE_RC_CONFIG_LINK_CAP_L0S BIT(10)
#define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
@@ -653,6 +655,13 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
status &= ~PCIE_RC_CONFIG_THP_CAP_NEXT_MASK;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_THP_CAP);
+ /* Clear L0s from RC's link cap */
+ if (of_property_read_bool(dev->of_node, "apsm-no-l0s")) {
+ status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LINK_CAP);
+ status &= ~PCIE_RC_CONFIG_LINK_CAP_L0S;
+ rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LINK_CAP);
+ }
+
rockchip_pcie_write(rockchip, 0x0, PCIE_RC_BAR_CONF);
rockchip_pcie_write(rockchip,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Sakari Ailus @ 2016-12-12 11:49 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <cc5229b9-0705-4189-39d5-7c3e0a96c008-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
> Hi Sakari,
>
> Thank you for the feedback.
>
> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
> > Hi Ramiro,
> >
> > Thank you for the patch.
> >
> > On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
> >> Add device tree documentation.
> >>
> >> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> ---
> >> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >> new file mode 100644
> >> index 0000000..4c91b3b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >> @@ -0,0 +1,19 @@
> >> +Omnivision OV5647 raw image sensor
> >> +---------------------------------
> >> +
> >> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
> >> +and CCI (I2C compatible) control bus.
> >> +
> >> +Required properties:
> >> +
> >> +- compatible : "ovti,ov5647";
> >> +- reg : I2C slave address of the sensor;
> >> +
> >> +The common video interfaces bindings (see video-interfaces.txt) should be
> >> +used to specify link to the image data receiver. The OV5647 device
> >> +node should contain one 'port' child node with an 'endpoint' subnode.
> >> +
> >> +Following properties are valid for the endpoint node:
> >> +
> >> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> >> + video-interfaces.txt. The sensor supports only two data lanes.
> >
> > Doesn't this sensor require a external clock, a reset GPIO and / or a
> > regulator or a few? Do you need data-lanes, unless you can change the order
> > or the number?
>
> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
> external clock but it's fixed and not controlled by SW. Should I add a property
> for this?
The sensor datasheet defines a power-up and power-down sequence for the
device. If you don't implement these sequences in the driver on a DT based
system, nothing suggests that they're implemented correctly. Could it be
that the boot loader simply enables the regulators or another device
requires them to be enabled?
I presume at least the reset GPIO should be controlled explicitly in order
to ensure correct function. Although hardware can be surprising: I have one
production system that has no reset GPIO for the sensor albeit the sensor
datasheet says that's part of the power up sequence.
>
> >
> > An example DT snippet wouldn't hurt.
>
> Sure, I can add a example snippet.
>
> >
>
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:39 UTC (permalink / raw)
To: linux-kernel
Cc: Sebastian Reichel, Rob Herring, Mark Rutland, linux-pm,
devicetree
In-Reply-To: <1481540424-19293-4-git-send-email-peda@axentia.se>
On 2016-12-12 12:00, Peter Rosin wrote:
> If several parallel bq24735 chargers have their ac-detect gpios wired
> together (or if only one of the parallel bq24735 chargers have its
> ac-detect pin wired to a gpio, and the others are assumed to react the
> same), then all driver instances need to check the same gpio. But the
> gpio subsystem does not allow sharing gpios, so handle that locally.
>
> However, only do this for the polling case, sharing is not supported if
> the ac detection is handled with interrupts.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/power/supply/bq24735-charger.c | 101 +++++++++++++++++++++++++++++----
> 1 file changed, 90 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
> index 3765806d5d46..3b21a064a9a7 100644
> --- a/drivers/power/supply/bq24735-charger.c
> +++ b/drivers/power/supply/bq24735-charger.c
> @@ -25,6 +25,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/power_supply.h>
> #include <linux/slab.h>
> @@ -43,12 +44,24 @@
> #define BQ24735_MANUFACTURER_ID 0xfe
> #define BQ24735_DEVICE_ID 0xff
>
> +struct bq24735;
> +
> +struct bq24735_shared {
> + struct list_head list;
> + struct bq24735 *owner;
> + struct gpio_desc *status_gpio;
> +};
> +
> +static struct mutex shared_lock;
Aww crap, that should of course be
static DEFINE_MUTEX(shared_lock);
Will fix in v2, but I'll wait for other feedback first. Why is it
impossible to see these things before hitting send?
Cheers,
peda
^ permalink raw reply
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Ramiro Oliveira @ 2016-12-12 11:39 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab, linux-kernel, linux-media, robh+dt, devicetree, davem,
gregkh, geert+renesas, akpm, linux, hverkuil, dheitmueller,
slongerbeam, lars, robert.jarzmik, pavel, pali.rohar,
sakari.ailus, mark.rutland, CARLOS.PALMINHA
In-Reply-To: <20161207223319.GZ16630@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thank you for the feedback.
On 12/7/2016 10:33 PM, Sakari Ailus wrote:
> Hi Ramiro,
>
> Thank you for the patch.
>
> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
>> Add device tree documentation.
>>
>> Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>> new file mode 100644
>> index 0000000..4c91b3b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>> @@ -0,0 +1,19 @@
>> +Omnivision OV5647 raw image sensor
>> +---------------------------------
>> +
>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
>> +and CCI (I2C compatible) control bus.
>> +
>> +Required properties:
>> +
>> +- compatible : "ovti,ov5647";
>> +- reg : I2C slave address of the sensor;
>> +
>> +The common video interfaces bindings (see video-interfaces.txt) should be
>> +used to specify link to the image data receiver. The OV5647 device
>> +node should contain one 'port' child node with an 'endpoint' subnode.
>> +
>> +Following properties are valid for the endpoint node:
>> +
>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>> + video-interfaces.txt. The sensor supports only two data lanes.
>
> Doesn't this sensor require a external clock, a reset GPIO and / or a
> regulator or a few? Do you need data-lanes, unless you can change the order
> or the number?
In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
external clock but it's fixed and not controlled by SW. Should I add a property
for this?
>
> An example DT snippet wouldn't hurt.
Sure, I can add a example snippet.
>
^ permalink raw reply
* Re: [PATCH v5 2/2] Add support for OV5647 sensor
From: Ramiro Oliveira @ 2016-12-12 11:39 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <20161207230138.GA16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hi Sakari
On 12/7/2016 11:01 PM, Sakari Ailus wrote:
> Hi Ramiro,
>
> On Mon, Dec 05, 2016 at 05:36:34PM +0000, Ramiro Oliveira wrote:
>> Add support for OV5647 sensor.
>>
>> Modes supported:
>> - 640x480 RAW 8
>>
>> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> ---
>> MAINTAINERS | 7 +
>> drivers/media/i2c/Kconfig | 12 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/ov5647.c | 866 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 886 insertions(+)
>> create mode 100644 drivers/media/i2c/ov5647.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 52cc077..72e828a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8923,6 +8923,13 @@ M: Harald Welte <laforge-TgoAw6mPHtdg9hUCZPvPmw@public.gmane.org>
>> S: Maintained
>> F: drivers/char/pcmcia/cm4040_cs.*
>>
>> +OMNIVISION OV5647 SENSOR DRIVER
>> +M: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> +L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +T: git git://linuxtv.org/media_tree.git
>> +S: Maintained
>> +F: drivers/media/i2c/ov5647.c
>> +
>> OMNIVISION OV7670 SENSOR DRIVER
>> M: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
>> L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index b31fa6f..c1b78e5 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -531,6 +531,18 @@ config VIDEO_OV2659
>> To compile this driver as a module, choose M here: the
>> module will be called ov2659.
>>
>> +config VIDEO_OV5647
>> + tristate "OmniVision OV5647 sensor support"
>> + depends on OF
>
> How does this driver depend on OF, other than matching the compatible
> string?
>
It doesn't, should I proceed diferently?
>> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> + depends on MEDIA_CAMERA_SUPPORT
>> + ---help---
>> + This is a Video4Linux2 sensor-level driver for the OmniVision
>> + OV5647 camera.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called ov5647.
>> +
>> config VIDEO_OV7640
>> tristate "OmniVision OV7640 sensor support"
>> depends on I2C && VIDEO_V4L2
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 92773b2..0d9014c 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
>> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
>> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
>> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
>> +obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
>> new file mode 100644
>> index 0000000..2aae806
>> --- /dev/null
>> +++ b/drivers/media/i2c/ov5647.c
>> @@ -0,0 +1,866 @@
>> +/*
>> + * A V4L2 driver for OmniVision OV5647 cameras.
>> + *
>> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
>> + * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2006-7 Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
>> + *
>> + * Copyright (C) 2016, Synopsys, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/i2c.h>
>> +#include <linux/delay.h>
>> +#include <linux/videodev2.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-mediabus.h>
>> +#include <media/v4l2-image-sizes.h>
>> +#include <media/v4l2-of.h>
>> +#include <linux/io.h>
>
> Alphabetical order, please.
>
>> +
>> +#define SENSOR_NAME "ov5647"
>> +
>> +#define OV5647_SW_RESET 0x1003
>> +#define OV5647_REG_CHIPID_H 0x300A
>> +#define OV5647_REG_CHIPID_L 0x300B
>> +
>> +#define REG_TERM 0xfffe
>> +#define VAL_TERM 0xfe
>> +#define REG_DLY 0xffff
>> +
>> +#define OV5647_ROW_START 0x01
>> +#define OV5647_ROW_START_MIN 0
>> +#define OV5647_ROW_START_MAX 2004
>> +#define OV5647_ROW_START_DEF 54
>> +
>> +#define OV5647_COLUMN_START 0x02
>> +#define OV5647_COLUMN_START_MIN 0
>> +#define OV5647_COLUMN_START_MAX 2750
>> +#define OV5647_COLUMN_START_DEF 16
>> +
>> +#define OV5647_WINDOW_HEIGHT 0x03
>> +#define OV5647_WINDOW_HEIGHT_MIN 2
>> +#define OV5647_WINDOW_HEIGHT_MAX 2006
>> +#define OV5647_WINDOW_HEIGHT_DEF 1944
>> +
>> +#define OV5647_WINDOW_WIDTH 0x04
>> +#define OV5647_WINDOW_WIDTH_MIN 2
>> +#define OV5647_WINDOW_WIDTH_MAX 2752
>> +#define OV5647_WINDOW_WIDTH_DEF 2592
>> +
>> +struct regval_list {
>> + u16 addr;
>> + u8 data;
>> +};
>> +
>> +struct cfg_array {
>> + struct regval_list *regs;
>> + int size;
>> +};
>> +
>> +struct sensor_win_size {
>> + int width;
>> + int height;
>> + unsigned int hoffset;
>> + unsigned int voffset;
>> + unsigned int hts;
>> + unsigned int vts;
>> + unsigned int pclk;
>> + unsigned int mipi_bps;
>> + unsigned int fps_fixed;
>> + unsigned int bin_factor;
>> + unsigned int intg_min;
>> + unsigned int intg_max;
>> + void *regs;
>> + int regs_size;
>> + int (*set_size)(struct v4l2_subdev *sd);
>> +};
>> +
>> +
>> +struct ov5647 {
>> + struct device *dev;
>> + struct v4l2_subdev sd;
>> + struct media_pad pad;
>> + struct mutex lock;
>> + struct v4l2_mbus_framefmt format;
>> + struct sensor_format_struct *fmt;
>> + unsigned int width;
>> + unsigned int height;
>> + unsigned int capture_mode;
>> + int hue;
>
> At least capture_mode and hue are unused. Please remove unused fields.
>
>> + struct v4l2_fract tpf;
>> + struct sensor_win_size *current_wins;
>> +};
>> +
>> +static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct ov5647, sd);
>> +}
>> +
>> +static struct regval_list sensor_oe_disable_regs[] = {
>> + {0x3000, 0x00},
>> + {0x3001, 0x00},
>> + {0x3002, 0x00},
>> +};
>> +
>> +static struct regval_list sensor_oe_enable_regs[] = {
>> + {0x3000, 0x0f},
>> + {0x3001, 0xff},
>> + {0x3002, 0xe4},
>> +};
>> +
>> +static struct regval_list ov5647_640x480[] = {
>
> Does this list expect a certain external clock frequency? If it does, should
> you check that the actual frequency matches with the expectation?
>
Yes. But like I said in the DT patch the external clock has a fixed frequency. I
can add a check if you thinks it's better.
>> + {0x0100, 0x00},
>> + {0x0103, 0x01},
>> + {0x3034, 0x08},
>> + {0x3035, 0x21},
>> + {0x3036, 0x46},
>> + {0x303c, 0x11},
>> + {0x3106, 0xf5},
>> + {0x3821, 0x07},
>> + {0x3820, 0x41},
>> + {0x3827, 0xec},
>> + {0x370c, 0x0f},
>> + {0x3612, 0x59},
>> + {0x3618, 0x00},
>> + {0x5000, 0x06},
>> + {0x5001, 0x01},
>> + {0x5002, 0x41},
>> + {0x5003, 0x08},
>> + {0x5a00, 0x08},
>> + {0x3000, 0x00},
>> + {0x3001, 0x00},
>> + {0x3002, 0x00},
>> + {0x3016, 0x08},
>> + {0x3017, 0xe0},
>> + {0x3018, 0x44},
>> + {0x301c, 0xf8},
>> + {0x301d, 0xf0},
>> + {0x3a18, 0x00},
>> + {0x3a19, 0xf8},
>> + {0x3c01, 0x80},
>> + {0x3b07, 0x0c},
>> + {0x380c, 0x07},
>> + {0x380d, 0x68},
>> + {0x380e, 0x03},
>> + {0x380f, 0xd8},
>> + {0x3814, 0x31},
>> + {0x3815, 0x31},
>> + {0x3708, 0x64},
>> + {0x3709, 0x52},
>> + {0x3808, 0x02},
>> + {0x3809, 0x80},
>> + {0x380a, 0x01},
>> + {0x380b, 0xE0},
>> + {0x3801, 0x00},
>> + {0x3802, 0x00},
>> + {0x3803, 0x00},
>> + {0x3804, 0x0a},
>> + {0x3805, 0x3f},
>> + {0x3806, 0x07},
>> + {0x3807, 0xa1},
>> + {0x3811, 0x08},
>> + {0x3813, 0x02},
>> + {0x3630, 0x2e},
>> + {0x3632, 0xe2},
>> + {0x3633, 0x23},
>> + {0x3634, 0x44},
>> + {0x3636, 0x06},
>> + {0x3620, 0x64},
>> + {0x3621, 0xe0},
>> + {0x3600, 0x37},
>> + {0x3704, 0xa0},
>> + {0x3703, 0x5a},
>> + {0x3715, 0x78},
>> + {0x3717, 0x01},
>> + {0x3731, 0x02},
>> + {0x370b, 0x60},
>> + {0x3705, 0x1a},
>> + {0x3f05, 0x02},
>> + {0x3f06, 0x10},
>> + {0x3f01, 0x0a},
>> + {0x3a08, 0x01},
>> + {0x3a09, 0x27},
>> + {0x3a0a, 0x00},
>> + {0x3a0b, 0xf6},
>> + {0x3a0d, 0x04},
>> + {0x3a0e, 0x03},
>> + {0x3a0f, 0x58},
>> + {0x3a10, 0x50},
>> + {0x3a1b, 0x58},
>> + {0x3a1e, 0x50},
>> + {0x3a11, 0x60},
>> + {0x3a1f, 0x28},
>> + {0x4001, 0x02},
>> + {0x4004, 0x02},
>> + {0x4000, 0x09},
>> + {0x4837, 0x24},
>> + {0x4050, 0x6e},
>> + {0x4051, 0x8f},
>> + {0x0100, 0x01},
>> +};
>> +
>> +struct sensor_format_struct;
>> +
>> +/**
>> + * @short I2C Write operation
>> + * @param[in] i2c_client I2C client
>> + * @param[in] reg register address
>> + * @param[in] val value to write
>> + * @return Error code
>> + */
>> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
>> +{
>> + int ret;
>> + unsigned char data[3] = { reg >> 8, reg & 0xff, val};
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = i2c_master_send(client, data, 3);
>> + if (ret != 3) {
>> + dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short I2C Read operation
>> + * @param[in] i2c_client I2C client
>> + * @param[in] reg register address
>> + * @param[out] val value read
>> + * @return Error code
>> + */
>> +static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
>> +{
>> + int ret;
>> + unsigned char data_w[2] = { reg >> 8, reg & 0xff };
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> +
>> + ret = i2c_master_send(client, data_w, 2);
>> +
>> + if (ret < 2) {
>> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> +
>> +
>> + ret = i2c_master_recv(client, val, 1);
>> +
>> + if (ret < 1) {
>> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> + return 0;
>> +}
>> +
>> +static int ov5647_write_array(struct v4l2_subdev *sd,
>> + struct regval_list *regs, int array_size)
>> +{
>> + int i = 0;
>> + int ret = 0;
>> +
>> + if (!regs)
>> + return -EINVAL;
>> +
>> + while (i < array_size) {
>> + ret = ov5647_write(sd, regs->addr, regs->data);
>> + if (ret < 0)
>> + return ret;
>> + i++;
>> + regs++;
>> + }
>> + return 0;
>> +}
>> +
>> +static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
>> +{
>> + u8 channel_id;
>> +
>> + ov5647_read(sd, 0x4814, &channel_id);
>> + channel_id &= ~(3 << 6);
>> + ov5647_write(sd, 0x4814, channel_id | (channel << 6));
>> +}
>> +
>> +void ov5647_stream_on(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ov5647_write(sd, 0x4202, 0x00);
>> + dev_dbg(&client->dev, "Stream on");
>> + ov5647_write(sd, 0x300D, 0x00);
>> +}
>> +
>> +void ov5647_stream_off(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ov5647_write(sd, 0x4202, 0x0f);
>> + dev_dbg(&client->dev, "Stream off");
>> + ov5647_write(sd, 0x300D, 0x01);
>> +}
>> +
>> +/****************************************************************************/
>> +
>> +/**
>> + * @short Set SW standby
>> + * @param[in] sd v4l2 sd
>> + * @param[in] stanby standby mode status (on or off)
>> + * @return Error code
>> + */
>> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
>> +{
>> + int ret;
>> + unsigned char rdval;
>> +
>> + ret = ov5647_read(sd, 0x0100, &rdval);
>> + if (ret != 0)
>> + return ret;
>> +
>> + if (standby)
>> + rdval &= 0xfe;
>> + else
>> + rdval |= 0x01;
>> +
>> + ret = ov5647_write(sd, 0x0100, rdval);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Store information about the video data format.
>> + */
>> +static struct sensor_format_struct {
>> + u8 *desc;
>> + u32 mbus_code;
>> + struct regval_list *regs;
>> + int regs_size;
>> + int bpp;
>
> At least desc and bpp are unused.
>
>> +} sensor_formats[] = {
>> + {
>> + .desc = "Raw RGB Bayer",
>> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
>> + .regs = ov5647_640x480,
>> + .regs_size = ARRAY_SIZE(ov5647_640x480),
>> + .bpp = 1
>> + },
>> +};
>> +#define N_FMTS ARRAY_SIZE(sensor_formats)
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Initialize sensor
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] val not used
>> + * @return Error code
>> + */
>> +static int __sensor_init(struct v4l2_subdev *sd)
>> +{
>> + int ret;
>> + u8 resetval;
>> + u8 rdval;
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + dev_dbg(&client->dev, "sensor init\n");
>> +
>> + ret = ov5647_read(sd, 0x0100, &rdval);
>> + if (ret != 0)
>> + return ret;
>> +
>> + ov5647_write(sd, 0x4800, 0x25);
>> + ov5647_stream_off(sd);
>> +
>> + ret = ov5647_write_array(sd, ov5647_640x480,
>> + ARRAY_SIZE(ov5647_640x480));
>> + if (ret < 0) {
>> + dev_err(&client->dev, "write sensor_default_regs error\n");
>> + return ret;
>> + }
>> +
>> + ov5647_set_virtual_channel(sd, 0);
>> +
>> + ov5647_read(sd, 0x0100, &resetval);
>> + if (!(resetval & 0x01)) {
>> + dev_err(&client->dev, "Device was in SW standby");
>> + ov5647_write(sd, 0x0100, 0x01);
>> + }
>> +
>> + ov5647_write(sd, 0x4800, 0x04);
>> + ov5647_stream_on(sd);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Control sensor power state
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] on Sensor power
>> + * @return Error code
>> + */
>> +static int sensor_power(struct v4l2_subdev *sd, int on)
>> +{
>> + int ret;
>> + struct ov5647 *ov5647 = to_state(sd);
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = 0;
>> + mutex_lock(&ov5647->lock);
>> +
>> + if (on) {
>> + dev_dbg(&client->dev, "OV5647 power on\n");
>> +
>> + ret = ov5647_write_array(sd, sensor_oe_enable_regs,
>> + ARRAY_SIZE(sensor_oe_enable_regs));
>> +
>> + ret = __sensor_init(sd);
>> +
>> + if (ret < 0)
>> + dev_err(&client->dev,
>> + "Camera not available, check Power\n");
>> + } else {
>> + dev_dbg(&client->dev, "OV5647 power off\n");
>> +
>> + dev_dbg(&client->dev, "disable oe\n");
>> + ret = ov5647_write_array(sd, sensor_oe_disable_regs,
>> + ARRAY_SIZE(sensor_oe_disable_regs));
>> +
>> + if (ret < 0)
>> + dev_dbg(&client->dev, "disable oe failed\n");
>> +
>> + ret = set_sw_standby(sd, true);
>> +
>> + if (ret < 0)
>> + dev_dbg(&client->dev, "soft stby failed\n");
>> +
>> + }
>> +
>> + mutex_unlock(&ov5647->lock);
>> +
>> + return ret;
>> +}
>> +
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> +/**
>> + * @short Get register value
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] reg register struct
>> + * @return Error code
>> + */
>> +static int sensor_get_register(struct v4l2_subdev *sd,
>> + struct v4l2_dbg_register *reg)
>> +{
>> + unsigned char val = 0;
>> + int ret;
>> +
>> + ret = ov5647_read(sd, reg->reg & 0xff, &val);
>> + if (ret != 0)
>> + return ret;
>> +
>> + reg->val = val;
>> + reg->size = 1;
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Set register value
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] reg register struct
>> + * @return Error code
>> + */
>> +static int sensor_set_register(struct v4l2_subdev *sd,
>> + const struct v4l2_dbg_register *reg)
>> +{
>> + return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
>> +}
>> +#endif
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Subdev core operations registration
>> + */
>> +static const struct v4l2_subdev_core_ops sensor_core_ops = {
>> + .s_power = sensor_power,
>
> The s_power() op will be called by the bridge (ISP) driver as well. You
> should expect that it may be enabled more than once before being disabled
> equal number of times.
>
Should I add an IF check to verify if the sensor is powered on before running
the power on/off routine.
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> + .g_register = sensor_get_register,
>> + .s_register = sensor_set_register,
>> +#endif
>> +};
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +
>> +
>> +/**
>> + * @short Enumerate available image formats
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] index index
>> + * @param[in] code MBUS Pixel code
>> + * @return Error code
>> + */
>> +static int sensor_enum_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> + if (code->pad || code->index >= N_FMTS)
>> + return -EINVAL;
>> +
>> + code->code = sensor_formats[code->index].mbus_code;
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Try frame format internal function
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fmt frame format
>> + * @return Error code
>> + */
>> +static int sensor_try_fmt_internal(struct v4l2_subdev *sd,
>> + struct v4l2_mbus_framefmt *fmt, struct sensor_format_struct **ret_fmt,
>> + struct sensor_win_size **ret_wsize)
>> +{
>> + int index;
>> +
>> + for (index = 0; index < N_FMTS; index++)
>> + if (sensor_formats[index].mbus_code == fmt->code)
>> + break;
>> +
>> + if (index >= N_FMTS)
>> + return -EINVAL;
>> +
>> + if (ret_fmt != NULL)
>> + *ret_fmt = sensor_formats + index;
>> +
>> + fmt->field = V4L2_FIELD_NONE;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Set frame format
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fmt frame format
>> + * @return Error code
>> + */
>> +static int sensor_s_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *fmt)
>> +{
>> + int ret;
>> + struct sensor_format_struct *sensor_fmt;
>> + struct sensor_win_size *wsize;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + ov5647_write_array(sd, sensor_oe_disable_regs,
>> + ARRAY_SIZE(sensor_oe_disable_regs));
>
> Should you check the error code here?
>
>> +
>> + ret = sensor_try_fmt_internal(sd, &fmt->format,
>> + &sensor_fmt, &wsize);
>
> Do you set wsize somewhere?
>
>> + if (ret)
>> + return ret;
>> +
>> + ov5647_write_array(sd, sensor_fmt->regs, sensor_fmt->regs_size);
>
> And here.
>
>> +
>> + ret = 0;
>
> ret was already zero.
>
>> +
>> + if (wsize->regs)
>> + ov5647_write_array(sd, wsize->regs, wsize->regs_size);
>> +
>> + if (wsize->set_size)
>> + wsize->set_size(sd);
>> +
>> + info->fmt = sensor_fmt;
>> + info->width = wsize->width;
>> + info->height = wsize->height;
>> +
>> + ov5647_write_array(sd, sensor_oe_enable_regs,
>> + ARRAY_SIZE(sensor_oe_enable_regs));
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Set stream parameters
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] parms stream parameters
>> + * @return Error code
>> + */
>> +static int sensor_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> +{
>> + struct v4l2_captureparm *cp = &parms->parm.capture;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + return -EINVAL;
>> +
>> + if (info->tpf.numerator == 0)
>> + return -EINVAL;
>> +
>> + info->capture_mode = cp->capturemode;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Get stream parameters
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] parms stream parameters
>> + * @return Error code
>> + */
>> +static int sensor_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> +{
>> + struct v4l2_captureparm *cp = &parms->parm.capture;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + return -EINVAL;
>> +
>> + memset(cp, 0, sizeof(struct v4l2_captureparm));
>> + cp->capability = V4L2_CAP_TIMEPERFRAME;
>> + cp->capturemode = info->capture_mode;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Subdev video operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_video_ops sensor_video_ops = {
>> + .s_parm = sensor_s_parm,
>> + .g_parm = sensor_g_parm,
>
> Please use the g/s_frame_interval() instead. That's what sub-device drivers
> generally use for the purpose.
>
>> +};
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Subdev operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_ops subdev_ops = {
>> + .core = &sensor_core_ops,
>> + .video = &sensor_video_ops,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * V4L2 subdev internal operations
>> + */
>> +
>> +/**
>> + * @short Detect camera version and model
>> + * @param[in] sd v4l2 subdev
>> + * @return Error code
>> + */
>> +int ov5647_detect(struct v4l2_subdev *sd)
>> +{
>> + unsigned char v;
>> + int ret;
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
>> + if (ret < 0)
>> + return ret;
>> + ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
>> + if (ret < 0)
>> + return ret;
>> + if (v != 0x56) {
>> + dev_err(&client->dev, "Wrong model version detected");
>> + return -ENODEV;
>> + }
>> + ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
>> + if (ret < 0)
>> + return ret;
>> + if (v != 0x47) {
>> + dev_err(&client->dev, "Wrong model version detected");
>> + return -ENODEV;
>> + }
>> +
>> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Detect if camera is registered
>> + * @param[in] sd v4l2 subdev
>> + * @return Error code
>> + */
>> +static int ov5647_registered(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + dev_info(&client->dev, "OV5647 detected at address 0x%02x\n",
>> + client->addr);
>
> I might omit this. If there's a need to debug this then the information
> should be printed by the framework instead using debug level messages.
>
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Open device
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fh v4l2 file handler
>> + * @return Error code
>> + */
>> +static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_mbus_framefmt *format =
>> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
>> + struct v4l2_rect *crop =
>> + v4l2_subdev_get_try_crop(sd, fh->pad, 0);
>> +
>> + crop->left = OV5647_COLUMN_START_DEF;
>> + crop->top = OV5647_ROW_START_DEF;
>> + crop->width = OV5647_WINDOW_WIDTH_DEF;
>> + crop->height = OV5647_WINDOW_HEIGHT_DEF;
>> +
>> + format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
>> +
>> + format->width = OV5647_WINDOW_WIDTH_DEF;
>> + format->height = OV5647_WINDOW_HEIGHT_DEF;
>> + format->field = V4L2_FIELD_NONE;
>> + format->colorspace = V4L2_COLORSPACE_SRGB;
>> +
>> + return sensor_power(sd, true);
>> +}
>> +
>> +/**
>> + * @short Open device
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fh v4l2 file handler
>> + * @return Error code
>> + */
>> +static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + return sensor_power(sd, false);
>> +}
>> +
>> +/**
>> + * @short Subdev internal operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
>> + .registered = ov5647_registered,
>> + .open = ov5647_open,
>> + .close = ov5647_close,
>> +};
>> +
>> +/**
>> + * @short Initialization routine - Entry point of the driver
>> + * @param[in] client pointer to the i2c client structure
>> + * @param[in] id pointer to the i2c device id structure
>> + * @return 0 on success and a negative number on failure
>> + */
>> +static int ov5647_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct ov5647 *sensor;
>> + int ret = 0;
>
> No need to initialise ret here.
>
>> + struct v4l2_subdev *sd;
>> +
>> + dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
>> +
>> + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>> + if (sensor == NULL)
>> + return -ENOMEM;
>> +
>> + mutex_init(&sensor->lock);
>> + sensor->dev = dev;
>> +
>> + sd = &sensor->sd;
>> + v4l2_i2c_subdev_init(sd, client, &subdev_ops);
>> + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> + sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
>> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
>> + ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
>> + if (ret < 0)
>> + goto mutex_remove;
>> +
>> + ret = ov5647_detect(sd);
>> + if (ret < 0)
>> + goto error;
>> +
>> + ret = v4l2_async_register_subdev(sd);
>> + if (ret < 0)
>> + goto error;
>> +
>> + return 0;
>> +error:
>> + media_entity_cleanup(&sd->entity);
>> +mutex_remove:
>> + mutex_destroy(&sensor->lock);
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Exit routine - Exit point of the driver
>> + * @param[in] client pointer to the i2c client structure
>> + * @return 0 on success and a negative number on failure
>> + */
>> +static int ov5647_remove(struct i2c_client *client)
>> +{
>> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
>> + struct ov5647 *ov5647 = to_state(sd);
>> +
>> + v4l2_async_unregister_subdev(&ov5647->sd);
>> + media_entity_cleanup(&ov5647->sd.entity);
>> + v4l2_device_unregister_subdev(sd);
>> + mutex_destroy(&ov5647->lock);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id ov5647_id[] = {
>> + { "ov5647", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ov5647_id);
>> +
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id ov5647_of_match[] = {
>> + { .compatible = "ovti,ov5647" },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ov5647_of_match);
>> +#endif
>> +
>> +/**
>> + * @short i2c driver structure
>> + */
>> +static struct i2c_driver ov5647_driver = {
>> + .driver = {
>> + .of_match_table = of_match_ptr(ov5647_of_match),
>> + .owner = THIS_MODULE,
>> + .name = "ov5647",
>> + },
>> + .probe = ov5647_probe,
>> + .remove = ov5647_remove,
>> + .id_table = ov5647_id,
>> +};
>> +
>> +module_i2c_driver(ov5647_driver);
>> +
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>");
>> +MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
>> +MODULE_LICENSE("GPL v2");
>
--
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 3/3] dts: hisi: add dts files for Hi3516CV300 demo board
From: Jiancheng Xue @ 2016-12-12 11:34 UTC (permalink / raw)
To: Pan Wen, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
Arnd Bergmann
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
howell.yang-C8/M+/jPZTeaMJb+Lgu22Q,
jalen.hsu-C8/M+/jPZTeaMJb+Lgu22Q,
lvkuanliang-C8/M+/jPZTeaMJb+Lgu22Q,
suwenping-C8/M+/jPZTeaMJb+Lgu22Q, raojun-C8/M+/jPZTeaMJb+Lgu22Q,
kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161017120705.3726-4-wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On 2016/10/17 20:07, Pan Wen wrote:
> Add dts files for Hi3516CV300 demo board.
>
> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
Could you help to review this patch, please?
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/hi3516cv300-demb.dts | 148 ++++++++++++
> arch/arm/boot/dts/hi3516cv300.dtsi | 397 +++++++++++++++++++++++++++++++++
> 3 files changed, 546 insertions(+)
> create mode 100644 arch/arm/boot/dts/hi3516cv300-demb.dts
> create mode 100644 arch/arm/boot/dts/hi3516cv300.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..1f25530 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_HIP01) += \
> dtb-$(CONFIG_ARCH_HIP04) += \
> hip04-d01.dtb
> dtb-$(CONFIG_ARCH_HISI) += \
> + hi3516cv300-demb.dtb \
> hi3519-demb.dtb
> dtb-$(CONFIG_ARCH_HIX5HD2) += \
> hisi-x5hd2-dkb.dtb
> diff --git a/arch/arm/boot/dts/hi3516cv300-demb.dts b/arch/arm/boot/dts/hi3516cv300-demb.dts
> new file mode 100644
> index 0000000..6a75cd6
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300-demb.dts
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +
> +/dts-v1/;
> +#include "hi3516cv300.dtsi"
> +
> +/ {
> + model = "Hisilicon Hi3516CV300 DEMO Board";
> + compatible = "hisilicon,hi3516cv300";
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + serial2 = &uart2;
> + i2c0 = &i2c_bus0;
> + i2c1 = &i2c_bus1;
> + spi0 = &spi_bus0;
> + spi1 = &spi_bus1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x80000000 0x10000000>;
> + };
> +};
> +
> +&dual_timer0 {
> + status = "okay";
> +};
> +
> +&watchdog {
> + status = "okay";
> +};
> +
> +&pwm {
> + status = "okay";
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&i2c_bus0 {
> + status = "okay";
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pmux>;
> +};
> +
> +&i2c_bus1 {
> + status = "okay";
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pmux>;
> +};
> +
> +&spi_bus0{
> + status = "disabled";
> + num-cs = <1>;
> + cs-gpios = <&gpio_chip0 6 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pmux>;
> +};
> +
> +&spi_bus1{
> + status = "okay";
> + num-cs = <2>;
> + cs-gpios = <&gpio_chip5 3 0>, <&gpio_chip5 4 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi1_pmux>;
> +};
> +
> +&fmc {
> + spi-nor@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <160000000>;
> + m25p,fast-read;
> + };
> +};
> +
> +&mdio {
> + phy0: phy@1 {
> + reg = <1>;
> + };
> +};
> +
> +&hisi_femac {
> + mac-address = [00 00 00 00 00 00];
> + phy-mode = "rmii";
> + phy-handle = <&phy0>;
> + hisilicon,phy-reset-delays-us = <10000 10000 150000>;
> +};
> +
> +&dmac {
> + status = "okay";
> +};
> +
> +&pmux {
> + i2c0_pmux: i2c0_pmux {
> + pinctrl-single,pins = <
> + 0x2c 0x3
> + 0x30 0x3>;
> + };
> +
> + i2c1_pmux: i2c1_pmux {
> + pinctrl-single,pins = <
> + 0x20 0x1
> + 0x24 0x1>;
> + };
> +
> + spi0_pmux: spi0_pmux {
> + pinctrl-single,pins = <
> + 0x28 0x1
> + 0x2c 0x1
> + 0x30 0x1
> + 0x34 0x1>;
> + };
> +
> + spi1_pmux: spi1_pmux {
> + pinctrl-single,pins = <
> + 0xc4 0x1
> + 0xc8 0x1
> + 0xcc 0x1
> + 0xd0 0x1
> + 0xd4 0x1>;
> + };
> +};
> diff --git a/arch/arm/boot/dts/hi3516cv300.dtsi b/arch/arm/boot/dts/hi3516cv300.dtsi
> new file mode 100644
> index 0000000..1da41ab
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300.dtsi
> @@ -0,0 +1,397 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "skeleton.dtsi"
> +#include <dt-bindings/clock/hi3516cv300-clock.h>
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,arm926ej-s";
> + reg = <0>;
> + };
> + };
> +
> + vic: interrupt-controller@10040000 {
> + compatible = "arm,pl190-vic";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + reg = <0x10040000 0x1000>;
> + };
> +
> + soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "simple-bus";
> + interrupt-parent = <&vic>;
> + ranges;
> +
> + clk_3m: clk_3m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <3000000>;
> + };
> +
> + clk_apb: clk_apb {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + };
> +
> + crg: clock-reset-controller@12010000 {
> + compatible = "hisilicon,hi3516cv300-crg";
> + reg = <0x12010000 0x1000>;
> + #clock-cells = <1>;
> + #reset-cells = <2>;
> + };
> +
> + sysctrl: system-controller@12020000 {
> + compatible = "hisilicon,hi3516cv300-sysctrl", "syscon";
> + reg = <0x12020000 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + reboot {
> + compatible = "syscon-reboot";
> + regmap = <&sysctrl>;
> + offset = <0x4>;
> + mask = <0xdeadbeef>;
> + };
> +
> + dual_timer0: dual_timer@12000000 {
> + compatible = "arm,sp804", "arm,primecell";
> + reg = <0x12000000 0x1000>;
> + interrupts = <3>;
> + clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> + clock-names = "timer0", "timer1", "apb_pclk";
> + status = "disabled";
> + };
> +
> + dual_timer1: dual_timer@12001000 {
> + compatible = "arm,sp804", "arm,primecell";
> + reg = <0x12001000 0x1000>;
> + interrupts = <4>;
> + clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> + clock-names = "timer0", "timer1", "apb_pclk";
> + status = "disabled";
> + };
> +
> + watchdog: watchdog@12080000 {
> + compatible = "arm,sp805-wdt", "arm,primecell";
> + arm,primecell-periphid = <0x00141805>;
> + reg = <0x12080000 0x1000>;
> + clocks = <&sysctrl HI3516CV300_WDT_CLK>,
> + <&crg HI3516CV300_APB_CLK>;
> + clock-names = "wdog_clk", "apb_pclk";
> + status = "disabled";
> + };
> +
> + pwm: pwm@12130000 {
> + compatible = "hisilicon,hi3516cv300-pwm",
> + "hisilicon,hibvt-pwm";
> + reg = <0x12130000 0x10000>;
> + clocks = <&crg HI3516CV300_PWM_CLK>;
> + resets = <&crg 0x38 0>;
> + #pwm-cells = <2>;
> + status = "disabled";
> + };
> +
> + uart0: uart@12100000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12100000 0x1000>;
> + interrupts = <5>;
> + clocks = <&crg HI3516CV300_UART0_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + uart1: uart@12101000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12101000 0x1000>;
> + interrupts = <30>;
> + clocks = <&crg HI3516CV300_UART1_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + uart2: uart@12102000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12102000 0x1000>;
> + interrupts = <25>;
> + clocks = <&crg HI3516CV300_UART2_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + i2c_bus0: i2c@12110000 {
> + compatible = "hisilicon,hi3516cv300-i2c",
> + "hisilicon,hibvt-i2c";
> + reg = <0x12110000 0x1000>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + status = "disabled";
> + };
> +
> + i2c_bus1: i2c@12112000 {
> + compatible = "hisilicon,hi3516cv300-i2c",
> + "hisilicon,hibvt-i2c";
> + reg = <0x12112000 0x1000>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + status = "disabled";
> + };
> +
> + spi_bus0: spi@12120000 {
> + compatible = "arm,pl022", "arm,primecell";
> + reg = <0x12120000 0x1000>;
> + interrupts = <6>;
> + clocks = <&crg HI3516CV300_SPI0_CLK>;
> + clock-names = "apb_pclk";
> + dmas = <&dmac 12 1>, <&dmac 13 2>;
> + dma-names = "rx", "tx";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + spi_bus1: spi@12121000 {
> + compatible = "arm,pl022", "arm,primecell";
> + reg = <0x12121000 0x1000>, <0x12030000 0x4>;
> + interrupts = <7>;
> + clocks = <&crg HI3516CV300_SPI1_CLK>;
> + clock-names = "apb_pclk";
> + dmas = <&dmac 14 1>, <&dmac 15 2>;
> + dma-names = "rx", "tx";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + fmc: spi-nor-controller@10000000 {
> + compatible = "hisilicon,fmc-spi-nor";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x10000000 0x1000>, <0x14000000 0x1000000>;
> + reg-names = "control", "memory";
> + clocks = <&crg HI3516CV300_FMC_CLK>;
> + assigned-clocks = <&crg HI3516CV300_FMC_CLK>;
> + assigned-clock-rates = <24000000>;
> + };
> +
> + mdio: mdio@10051100 {
> + compatible = "hisilicon,hisi-femac-mdio";
> + reg = <0x10051100 0x10>;
> + clocks = <&crg HI3516CV300_ETH_CLK>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + hisi_femac: ethernet@10090000 {
> + compatible = "hisilicon,hi3516cv300-femac",
> + "hisilicon,hisi-femac-v2";
> + reg = <0x10050000 0x1000>,<0x10051300 0x200>;
> + interrupts = <12>;
> + clocks = <&crg HI3516CV300_ETH_CLK>;
> + resets = <&crg 0xec 0>, <&crg 0xec 3>;
> + reset-names = "mac","phy";
> + };
> +
> + dmac: dma-controller@10030000 {
> + compatible = "arm,pl080", "arm,primecell";
> + reg = <0x10030000 0x1000>;
> + interrupts = <14>;
> + clocks = <&crg HI3516CV300_DMAC_CLK>;
> + clock-names = "apb_pclk";
> + lli-bus-interface-ahb1;
> + lli-bus-interface-ahb2;
> + mem-bus-interface-ahb1;
> + mem-bus-interface-ahb2;
> + memcpy-burst-size = <256>;
> + memcpy-bus-width = <32>;
> + #dma-cells = <2>;
> + status = "disabled";
> + };
> +
> + gpio_chip0: gpio@12140000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12140000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 61 2>,
> + <&pmux 4 11 1>,
> + <&pmux 5 10 1>,
> + <&pmux 6 13 2>;
> +
> + status = "disabled";
> + };
> +
> + gpio_chip1: gpio@12141000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12141000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 16 7>,
> + <&pmux 7 0 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip2: gpio@12142000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12142000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 46 1>,
> + <&pmux 1 45 1>,
> + <&pmux 2 44 1>,
> + <&pmux 3 43 1>,
> + <&pmux 4 39 1>,
> + <&pmux 5 38 1>,
> + <&pmux 6 40 1>,
> + <&pmux 7 48 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip3: gpio@12143000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12143000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 37 1>,
> + <&pmux 1 36 1>,
> + <&pmux 2 35 1>,
> + <&pmux 3 34 1>,
> + <&pmux 4 23 2>,
> + <&pmux 6 8 2>;
> + status = "disabled";
> + };
> +
> + gpio_chip4: gpio@12144000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12144000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 27 1>,
> + <&pmux 1 26 1>,
> + <&pmux 2 31 1>,
> + <&pmux 3 30 1>,
> + <&pmux 4 28 2>,
> + <&pmux 6 33 1>,
> + <&pmux 7 32 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip5: gpio@12145000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12145000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 53 1>,
> + <&pmux 1 51 2>,
> + <&pmux 3 50 1>,
> + <&pmux 4 49 1>,
> + <&pmux 5 47 1>,
> + <&pmux 6 40 2>;
> + status = "disabled";
> + };
> +
> + gpio_chip6: gpio@12146000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12146000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 7 1>,
> + <&pmux 1 6 1>,
> + <&pmux 2 4 1>,
> + <&pmux 3 5 1>,
> + <&pmux 4 15 1>,
> + <&pmux 5 1 3>;
> + status = "disabled";
> + };
> +
> + gpio_chip7: gpio@12147000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12147000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 1 55 6>,
> + <&pmux 7 25 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip8: gpio@12148000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12148000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 63 3>,
> + <&pmux 3 12 1>;
> + status = "disabled";
> + };
> +
> + pmux: pinmux@12040000 {
> + compatible = "pinctrl-single";
> + reg = <0x12040000 0x108>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #gpio-range-cells = <3>;
> + ranges;
> +
> + pinctrl-single,register-width = <32>;
> + pinctrl-single,function-mask = <7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 54 0
> + &range 55 6 1 &range 61 5 0>;
> +
> + range: gpio-range {
> + #pinctrl-single,gpio-range-cells = <3>;
> + };
> + };
> +
> + pconf: pinconf@12040800 {
> + compatible = "pinconf-single";
> + reg = <0x12040800 0x130>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + pinctrl-single,register-width = <32>;
> + };
> + };
> +};
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/4] net: hix5hd2_gmac: add generic compatible string
From: Dongpo Li @ 2016-12-12 11:16 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland-5wv7dgnIgG8, mturquette-rdvid1DuHRBWk0Htik3J/w,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
salil.mehta-hv44wF8Li93QT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
arnd-r2nGTMty4D4, andrew-g2DYL2Zd6BY,
xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
benjamin.chenhao-C8/M+/jPZTeaMJb+Lgu22Q,
caizhiyong-C8/M+/jPZTeaMJb+Lgu22Q, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161209223521.5dnj7l44e663sntl@rob-hp-laptop>
Hi Rob,
On 2016/12/10 6:35, Rob Herring wrote:
> On Mon, Dec 05, 2016 at 09:27:58PM +0800, Dongpo Li wrote:
>> The "hix5hd2" is SoC name, add the generic ethernet driver name.
>> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
>> the SG/TXCSUM/TSO/UFO features.
>>
>> Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>> ---
>> .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt | 9 +++++++--
>> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 15 +++++++++++----
>> 2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> index 75d398b..75920f0 100644
>> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> @@ -1,7 +1,12 @@
>> Hisilicon hix5hd2 gmac controller
>>
>> Required properties:
>> -- compatible: should be "hisilicon,hix5hd2-gmac".
>> +- compatible: should contain one of the following SoC strings:
>> + * "hisilicon,hix5hd2-gemac"
>> + * "hisilicon,hi3798cv200-gemac"
>> + and one of the following version string:
>> + * "hisilicon,hisi-gemac-v1"
>> + * "hisilicon,hisi-gemac-v2"
>
> What combinations are valid? I assume both chips don't have both v1 and
> v2. 2 SoCs and 2 versions so far, I don't think there is much point to
> have the v1 and v2 compatible strings.
>
The v1 and v2 are generic MAC compatible strings, many HiSilicon SoCs may
use the same MAC version. For example,
hix5hd2, hi3716cv200 SoCs use the v1 MAC version,
hi3798cv200, hi3516a SoCs use the v2 MAC version,
and there may be more SoCs added in future.
So I think the generic compatible strings are okay here.
Should I add the hi3716cv200, hi3516a SoCs compatible here?
Do you have any good advice?
>> - reg: specifies base physical address(s) and size of the device registers.
>> The first region is the MAC register base and size.
>> The second region is external interface control register.
>> @@ -20,7 +25,7 @@ Required properties:
>>
>> Example:
>> gmac0: ethernet@f9840000 {
>> - compatible = "hisilicon,hix5hd2-gmac";
>> + compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>
> You can't just change compatible strings.
>
Okay, maybe I should name all the compatible string with the suffix "-gmac" instead of
"-gemac". This can keep the compatible strings with the same suffix. Is this okay?
Can I just add the generic compatible string without changing the SoCs compatible string?
Like following:
gmac0: ethernet@f9840000 {
- compatible = "hisilicon,hix5hd2-gmac";
+ compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
>> reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
>> interrupts = <0 71 4>;
>> #address-cells = <1>;
>
> .
>
Regards,
Dongpo
.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Wolfram Sang @ 2016-12-12 11:10 UTC (permalink / raw)
To: Kachalov Anton
Cc: Brendan Higgins, vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org,
clg-Bxea+6Xhats@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <825541481540788-Mr5SgJCofHtxpj1cXAZ9Bg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> >> + /* Switch from master mode to slave mode. */
> >> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> >> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> >> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> >> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> >
> > Can't the hardware work both as master and slave on the same bus?
>
> The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
Thanks! Then the driver should support this. Maybe it is an idea to
first upstream the master support and add the slave support
incrementally?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Kachalov Anton @ 2016-12-12 11:06 UTC (permalink / raw)
To: Wolfram Sang, Brendan Higgins
Cc: vz@mleia.com, clg@kaod.org, robh+dt@kernel.org,
mark.rutland@arm.com, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, joel@jms.id.au,
openbmc@lists.ozlabs.org
In-Reply-To: <20161211222622.GK2552@katana>
12.12.2016, 01:26, "Wolfram Sang" <wsa@the-dreams.de>:
> On Tue, Nov 29, 2016 at 05:00:17PM -0800, Brendan Higgins wrote:
>> Added initial master and slave support for Aspeed I2C controller.
>> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
>> Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>
> BTW first the bindings patch please, then the driver.
>
> And one seperate question I just stumbled over:
>
>> + /* Switch from master mode to slave mode. */
>> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
>> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
>> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
>> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
>
> Can't the hardware work both as master and slave on the same bus?
The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
^ permalink raw reply
* [PATCH 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
In-Reply-To: <1481540424-19293-1-git-send-email-peda@axentia.se>
If several parallel bq24735 chargers have their ac-detect gpios wired
together (or if only one of the parallel bq24735 chargers have its
ac-detect pin wired to a gpio, and the others are assumed to react the
same), then all driver instances need to check the same gpio. But the
gpio subsystem does not allow sharing gpios, so handle that locally.
However, only do this for the polling case, sharing is not supported if
the ac detection is handled with interrupts.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/power/supply/bq24735-charger.c | 101 +++++++++++++++++++++++++++++----
1 file changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 3765806d5d46..3b21a064a9a7 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -25,6 +25,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/power_supply.h>
#include <linux/slab.h>
@@ -43,12 +44,24 @@
#define BQ24735_MANUFACTURER_ID 0xfe
#define BQ24735_DEVICE_ID 0xff
+struct bq24735;
+
+struct bq24735_shared {
+ struct list_head list;
+ struct bq24735 *owner;
+ struct gpio_desc *status_gpio;
+};
+
+static struct mutex shared_lock;
+static LIST_HEAD(shared_list);
+
struct bq24735 {
struct power_supply *charger;
struct power_supply_desc charger_desc;
struct i2c_client *client;
struct bq24735_platform *pdata;
struct mutex lock;
+ struct bq24735_shared *shared;
struct gpio_desc *status_gpio;
struct delayed_work poll;
u32 poll_interval;
@@ -346,6 +359,65 @@ static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client)
return pdata;
}
+static struct gpio_desc *bq24735_get_status_gpio(struct bq24735 *charger)
+{
+ struct device *dev = &charger->client->dev;
+ struct bq24735_shared *shared;
+ int gpio;
+ struct list_head *pos;
+
+ if (!of_property_read_bool(dev->of_node, "ti,ac-detect-gpios"))
+ return NULL;
+
+ gpio = of_get_named_gpio(dev->of_node, "ti,ac-detect-gpios", 0);
+ if (gpio < 0)
+ return ERR_PTR(gpio);
+
+ mutex_lock(&shared_lock);
+ list_for_each(pos, &shared_list) {
+ shared = list_entry(pos, struct bq24735_shared, list);
+ if (gpio != desc_to_gpio(shared->status_gpio))
+ continue;
+
+ get_device(&shared->owner->client->dev);
+ dev_dbg(dev, "sharing gpio with %s\n",
+ shared->owner->pdata->name);
+ charger->shared = shared;
+ mutex_unlock(&shared_lock);
+ return shared->status_gpio;
+ }
+
+ shared = devm_kzalloc(dev, sizeof(*shared), GFP_KERNEL);
+ if (!shared) {
+ mutex_unlock(&shared_lock);
+ return ERR_PTR(-ENOMEM);
+ }
+ shared->owner = charger;
+ shared->status_gpio = gpiod_get(dev, "ti,ac-detect", GPIOD_IN);
+ if (!IS_ERR(shared->status_gpio)) {
+ charger->shared = shared;
+ list_add(&shared->list, &shared_list);
+ }
+ mutex_unlock(&shared_lock);
+
+ return shared->status_gpio;
+}
+
+static void bq24735_put_status_gpio(struct bq24735 *charger)
+{
+ if (!charger->shared)
+ return;
+
+ mutex_lock(&shared_lock);
+ if (charger->shared->owner != charger) {
+ put_device(&charger->shared->owner->client->dev);
+ } else {
+ list_del(&charger->shared->list);
+ gpiod_put(charger->shared->status_gpio);
+ }
+ mutex_unlock(&shared_lock);
+}
+
static int bq24735_charger_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -402,9 +474,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
i2c_set_clientdata(client, charger);
- charger->status_gpio = devm_gpiod_get_optional(&client->dev,
- "ti,ac-detect",
- GPIOD_IN);
+ charger->status_gpio = bq24735_get_status_gpio(charger);
if (IS_ERR(charger->status_gpio)) {
ret = PTR_ERR(charger->status_gpio);
dev_err(&client->dev, "Getting gpio failed: %d\n", ret);
@@ -416,28 +486,30 @@ static int bq24735_charger_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&client->dev, "Failed to read manufacturer id : %d\n",
ret);
- return ret;
+ goto out;
} else if (ret != 0x0040) {
dev_err(&client->dev,
"manufacturer id mismatch. 0x0040 != 0x%04x\n", ret);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out;
}
ret = bq24735_read_word(client, BQ24735_DEVICE_ID);
if (ret < 0) {
dev_err(&client->dev, "Failed to read device id : %d\n", ret);
- return ret;
+ goto out;
} else if (ret != 0x000B) {
dev_err(&client->dev,
"device id mismatch. 0x000b != 0x%04x\n", ret);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out;
}
}
ret = bq24735_config_charger(charger);
if (ret < 0) {
dev_err(&client->dev, "failed in configuring charger");
- return ret;
+ goto out;
}
/* check for AC adapter presence */
@@ -445,7 +517,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
ret = bq24735_enable_charging(charger);
if (ret < 0) {
dev_err(&client->dev, "Failed to enable charging\n");
- return ret;
+ goto out;
}
}
@@ -455,7 +527,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
ret = PTR_ERR(charger->charger);
dev_err(&client->dev, "Failed to register power supply: %d\n",
ret);
- return ret;
+ goto out;
}
if (client->irq) {
@@ -470,7 +542,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
dev_err(&client->dev,
"Unable to register IRQ %d err %d\n",
client->irq, ret);
- return ret;
+ goto out;
}
} else if (charger->status_gpio) {
ret = of_property_read_u32(client->dev.of_node,
@@ -487,6 +559,11 @@ static int bq24735_charger_probe(struct i2c_client *client,
}
return 0;
+
+out:
+ bq24735_put_status_gpio(charger);
+
+ return ret;
}
static int bq24735_charger_remove(struct i2c_client *client)
@@ -496,6 +573,8 @@ static int bq24735_charger_remove(struct i2c_client *client)
if (charger->poll_interval)
cancel_delayed_work_sync(&charger->poll);
+ bq24735_put_status_gpio(charger);
+
return 0;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 2/3] power: supply: bq24735-charger: optionally poll the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
In-Reply-To: <1481540424-19293-1-git-send-email-peda@axentia.se>
If the ac-detect gpio does not support interrupts, provide a fallback
to poll the gpio at a configurable interval.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../bindings/power/supply/ti,bq24735.txt | 2 +
drivers/power/supply/bq24735-charger.c | 50 +++++++++++++++++++---
2 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
index 3bf55757ceec..59c4dde589cf 100644
--- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
+++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
@@ -25,6 +25,8 @@ Optional properties :
- ti,external-control : Indicates that the charger is configured externally
and that the host should not attempt to enable/disable charging or set the
charge voltage/current.
+ - ti,poll-interval-ms : In case there is no interrupts specified, poll AC
+ presense on the ti,ac-detect-gpios GPIO with this interval.
Example:
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 1d5c9206e0ed..3765806d5d46 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -50,6 +50,8 @@ struct bq24735 {
struct bq24735_platform *pdata;
struct mutex lock;
struct gpio_desc *status_gpio;
+ struct delayed_work poll;
+ u32 poll_interval;
bool charging;
};
@@ -209,11 +211,8 @@ static int bq24735_charger_is_charging(struct bq24735 *charger)
return !(ret & BQ24735_CHG_OPT_CHARGE_DISABLE);
}
-static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+static void bq24735_update(struct bq24735 *charger)
{
- struct power_supply *psy = devid;
- struct bq24735 *charger = to_bq24735(psy);
-
mutex_lock(&charger->lock);
if (charger->charging && bq24735_charger_is_present(charger))
@@ -223,11 +222,29 @@ static irqreturn_t bq24735_charger_isr(int irq, void *devid)
mutex_unlock(&charger->lock);
- power_supply_changed(psy);
+ power_supply_changed(charger->charger);
+}
+
+static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+{
+ struct power_supply *psy = devid;
+ struct bq24735 *charger = to_bq24735(psy);
+
+ bq24735_update(charger);
return IRQ_HANDLED;
}
+static void bq24735_poll(struct work_struct *work)
+{
+ struct bq24735 *charger = container_of(work, struct bq24735, poll.work);
+
+ bq24735_update(charger);
+
+ schedule_delayed_work(&charger->poll,
+ msecs_to_jiffies(charger->poll_interval));
+}
+
static int bq24735_charger_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -455,11 +472,33 @@ static int bq24735_charger_probe(struct i2c_client *client,
client->irq, ret);
return ret;
}
+ } else if (charger->status_gpio) {
+ ret = of_property_read_u32(client->dev.of_node,
+ "ti,poll-interval-ms",
+ &charger->poll_interval);
+ if (ret)
+ return 0;
+ if (!charger->poll_interval)
+ return 0;
+
+ INIT_DELAYED_WORK(&charger->poll, bq24735_poll);
+ schedule_delayed_work(&charger->poll,
+ msecs_to_jiffies(charger->poll_interval));
}
return 0;
}
+static int bq24735_charger_remove(struct i2c_client *client)
+{
+ struct bq24735 *charger = i2c_get_clientdata(client);
+
+ if (charger->poll_interval)
+ cancel_delayed_work_sync(&charger->poll);
+
+ return 0;
+}
+
static const struct i2c_device_id bq24735_charger_id[] = {
{ "bq24735-charger", 0 },
{}
@@ -478,6 +517,7 @@ static struct i2c_driver bq24735_charger_driver = {
.of_match_table = bq24735_match_ids,
},
.probe = bq24735_charger_probe,
+ .remove = bq24735_charger_remove,
.id_table = bq24735_charger_id,
};
--
2.1.4
^ permalink raw reply related
* [PATCH 1/3] power: supply: bq24735-charger: simplify register update to stop charging
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1481540424-19293-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Providing value bits outside of the mask is pointless.
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
drivers/power/supply/bq24735-charger.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index eb7783b42e0a..1d5c9206e0ed 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -111,8 +111,7 @@ static inline int bq24735_enable_charging(struct bq24735 *charger)
return 0;
return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
- BQ24735_CHG_OPT_CHARGE_DISABLE,
- ~BQ24735_CHG_OPT_CHARGE_DISABLE);
+ BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
}
static inline int bq24735_disable_charging(struct bq24735 *charger)
--
2.1.4
--
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 0/3] bq24735-charger: allow gpio polling and sharing
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
Hi!
I have a board that features a couple of parallel bq24735 chargers.
However, only one of the chargers has its ACOK pin wired to a gpio,
the other parallel chargers are expected to just behave the same
as the charger that has been singled out like this. Another thing
with the board is that the gpio is not capable of generating an
interrupt.
This series fixes things for me (ok, the first patch was just a
fix for a thing that initially had me confused for a bit, and is
totally unrelated. Ignore if you want to, it's basically just churn).
One thing that I wonder about is if anything should be done to
prevent unloading of the instance that shares its gpio? I thought
about adding a device_link_add() call, but am unsure if that would
be approprite? I'm not unloading drivers at all so it's a total
non-issue for me...
Cheers,
Peter
Peter Rosin (3):
power: supply: bq24735-charger: simplify register update to stop
charging
power: supply: bq24735-charger: optionally poll the ac-detect gpio
power: supply: bq24735-charger: allow chargers to share the ac-detect
gpio
.../bindings/power/supply/ti,bq24735.txt | 2 +
drivers/power/supply/bq24735-charger.c | 154 ++++++++++++++++++---
2 files changed, 138 insertions(+), 18 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH V2 2/2] PM / OPP: Introduce domain-performance-state binding to OPP nodes
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot,
Rob Herring, Mark Rutland, Kevin Hilman, Ulf Hansson, Lina Iyer,
devicetree, Nayak Rajendra, Viresh Kumar
In-Reply-To: <cover.1481539827.git.viresh.kumar@linaro.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their nodes directly.
But if the device needs the capability of switching to different domain
performance states, as they may need to support different clock rates,
then the per OPP node can be used to contain that information.
This patch introduces the domain-performance-state (already defined by
Power Domain bindings) to the per OPP node.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 59 +++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 9f5ca4457b5f..43eba7c9fc58 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -154,6 +154,9 @@ properties.
- status: Marks the node enabled/disabled.
+- domain-performance-state: A phandle of a Performance state node as defined by
+ ../power/power_domain.txt binding document.
+
Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
/ {
@@ -528,3 +531,59 @@ Example 5: opp-supported-hw
};
};
};
+
+Example 7: domain-Performance-state:
+(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
+
+/ {
+ foo_domain: power-controller@12340000 {
+ compatible = "foo,power-controller";
+ reg = <0x12340000 0x1000>;
+ #power-domain-cells = <0>;
+ domain-performance-states = <&domain_perf_states>;
+ };
+
+ domain_perf_states: performance_states {
+ compatible = "domain-performance-state";
+ domain_perf_state1: pstate@1 {
+ performance-level = <1>;
+ domain-microvolt = <970000 975000 985000>;
+ };
+ domain_perf_state2: pstate@2 {
+ performance-level = <2>;
+ domain-microvolt = <1000000 1075000 1085000>;
+ };
+ }
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ compatible = "arm,cortex-a9";
+ reg = <0>;
+ clocks = <&clk_controller 0>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
+ power-domains = <&foo_domain>;
+ };
+ };
+
+ cpu0_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp@1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ domain-performance-state = <&domain_perf_state1>;
+ };
+ opp@1100000000 {
+ opp-hz = /bits/ 64 <1100000000>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+ opp@1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+ };
+};
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V2 1/2] PM / Domains: Introduce domain-performance-states binding
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Stephen Boyd,
Nishanth Menon, Vincent Guittot, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Lina Iyer, devicetree, Nayak Rajendra,
Viresh Kumar
In-Reply-To: <cover.1481539827.git.viresh.kumar@linaro.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
This patch adds binding to describe the performance states of a power
domain.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their node directly. Otherwise the
consumers can define their requirements with help of other
infrastructure, for example the OPP table.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../devicetree/bindings/power/power_domain.txt | 69 ++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad937da..a456e0dc04e0 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -38,6 +38,40 @@ phandle arguments (so called PM domain specifiers) of length specified by the
domain's idle states. In the absence of this property, the domain would be
considered as capable of being powered-on or powered-off.
+- domain-performance-states : A phandle of the performance states node, which
+ defines all the performance states associated with a power
+ domain.
+ The domain-performance-states property reflects the performance states of this
+ PM domain and not the performance states of the devices or sub-domains in the
+ PM domain. Devices and sub-domains have their own performance states, which
+ are dependent on the performance state of the PM domain.
+
+* PM domain performance states node
+
+This describes the performance states of a PM domain.
+
+Required properties:
+- compatible: Allow performance states to express their compatibility. It should
+ be: "domain-performance-state".
+
+- Performance state nodes: This node shall have one or more "Performance State"
+ nodes.
+
+* Performance state node
+
+Required properties:
+- performance-level: A positive integer value representing the performance level
+ associated with a performance state. The integer value '1' represents the
+ lowest performance level and the highest value represents the highest
+ performance level.
+
+Optional properties:
+- domain-microvolt: voltage in micro Volts.
+
+ A single regulator's voltage is specified with an array of size one or three.
+ Single entry is for target voltage and three entries are for <target min max>
+ voltages.
+
Example:
power: power-controller@12340000 {
@@ -118,4 +152,39 @@ The node above defines a typical PM domain consumer device, which is located
inside a PM domain with index 0 of a power controller represented by a node
with the label "power".
+Optional properties:
+- domain-performance-state: A phandle of a Performance state node.
+
+Example:
+
+ parent: power-controller@12340000 {
+ compatible = "foo,power-controller";
+ reg = <0x12340000 0x1000>;
+ #power-domain-cells = <0>;
+ domain-performance-states = <&domain_perf_states>;
+ };
+
+ domain_perf_states: performance_states {
+ compatible = "domain-performance-state";
+ domain_perf_state1: pstate@1 {
+ performance-level = <1>;
+ domain-microvolt = <970000 975000 985000>;
+ };
+ domain_perf_state2: pstate@2 {
+ performance-level = <2>;
+ domain-microvolt = <1000000 1075000 1085000>;
+ };
+ domain_perf_state3: pstate@3 {
+ performance-level = <3>;
+ domain-microvolt = <1100000 1175000 1185000>;
+ };
+ }
+
+ leaky-device@12350000 {
+ compatible = "foo,i-leak-current";
+ reg = <0x12350000 0x1000>;
+ power-domains = <&power 0>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+
[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V2 0/2] PM / Domains / OPP: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Stephen Boyd,
Nishanth Menon, Vincent Guittot, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Lina Iyer, devicetree, Nayak Rajendra,
Viresh Kumar
Hello,
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
We had some discussions about it in the past on the PM list [1], which is
followed by discussions during the LPC. The outcome of all that was that we
should extend Power Domain framework to support active state power management
as well.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
To get a complete picture of the proposed plan, following is what we
need to do:
- Create DT bindings to get domain performance state information for the
platforms.
- Enhance OPP framework to parse these and call into the PM Qos
framework with a performance state request.
- Enhance PM Qos framework to provide the API to be used by consumers
(or OPP framework) and pass it on to the (Generic) Power Domain
framework.
- Enhance Generic Power Domain framework to accept such requests,
accumulate all belonging to a single power domain and call domain
driver specific callback with the performance state we want for the
domain.
- The domain driver shall then, in a platform specific way, set the
requested performance level.
- Note that these features are applicable to the CPU, GPU and other IIO
or non-IIO devices.
- There can be cases where a device can choose between multiple power
domains based on what performance level we want for the device. In
such cases, we should represent the multiplexer with a separate power
domain. In effect, the device (or OPP table) will correspond to a
single power domain, but the backend driver of that domain shall
implement the multiplexing functionality.
This patchset implements the very first part of this chain and
introduces a new optional property for the consumers of the
power-domains: domain-performance-state. This property can be used
directly by the consumer or its OPP table.
V1->V2:
- The performance states get their own nodes as they can have multiple
values.
- Allow optional property domain-microvolt for the performance states
--
viresh
[1] https://marc.info/?l=linux-pm&m=147747923708075&w=2
Viresh Kumar (2):
PM / Domains: Introduce domain-performance-states binding
PM / OPP: Introduce domain-performance-state binding to OPP nodes
Documentation/devicetree/bindings/opp/opp.txt | 59 ++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 69 ++++++++++++++++++++++
2 files changed, 128 insertions(+)
--
2.7.1.410.g6faf27b
^ permalink raw reply
* Re: [PATCH 3/8] rtc: add STM32 RTC driver
From: Amelie DELAUNAY @ 2016-12-12 10:19 UTC (permalink / raw)
To: alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org
Cc: a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Alexandre TORGUE, linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Gabriel FERNANDEZ
In-Reply-To: <bc5c308d-472a-bf05-89b8-6d813fb5321d-qxv4g6HH51o@public.gmane.org>
Hi,
Thanks for the review.
On 12/07/2016 08:08 PM, Alexandre Belloni wrote:
> Hi,
>
> It seems mostly fine.
>
> On 02/12/2016 at 15:09:56 +0100, Amelie Delaunay wrote :
>> This patch adds support for the STM32 RTC.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
>> ---
>> drivers/rtc/Kconfig | 10 +
>> drivers/rtc/Makefile | 1 +
>> drivers/rtc/rtc-stm32.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 788 insertions(+)
>> create mode 100644 drivers/rtc/rtc-stm32.c
>>
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index e859d14..dd8b218 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -1706,6 +1706,16 @@ config RTC_DRV_PIC32
>> This driver can also be built as a module. If so, the module
>> will be called rtc-pic32
>>
>> +config RTC_DRV_STM32
>> + tristate "STM32 On-Chip RTC"
>> + depends on ARCH_STM32
>
> Can you add COMPILE_TEST? Looking at it, nothing seemed to be
> architecture specific and this nicely increases compile test coverage.
>
> It should also probably select REGMAP_MMIO.
>
Ok.
>> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
>> new file mode 100644
>> index 0000000..9e710ff
>> --- /dev/null
>> +++ b/drivers/rtc/rtc-stm32.c
>> @@ -0,0 +1,777 @@
>> +/*
>> + * Copyright (C) Amelie Delaunay 2015
>> + * Author: Amelie Delaunay <adelaunay.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This differs from your SoB. I don't really care but it seems odd.
>
Yes, I've already changed it in my upcoming V2!
>> + * License terms: GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/bcd.h>
>> +#include <linux/clk.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/rtc.h>
>> +#include <linux/spinlock.h>
>> +
>
> I have the feeling that some of those headers are not necessary maybe
> some cleanup should be done.
>
Ok I'll have a look.
>> +static struct regmap *dbp;
>> +
>> +struct stm32_rtc {
>> + struct rtc_device *rtc_dev;
>> + void __iomem *base;
>> + struct clk *pclk;
>> + struct clk *ck_rtc;
>> + unsigned int clksrc;
>> + spinlock_t lock; /* Protects registers accesses */
>
> That comment makes checkpatch happy but is not super useful :) Anyway...
>
Lack of inspiration :)
>> +static irqreturn_t stm32_rtc_alarm_irq(int irq, void *dev_id)
>> +{
>
> ...can you make that one a threaded IRQ? If that's the case, just take
> the rtc_device mutex here and remove the spinlock. All the other
> function are already protected.
>
Ok I'll study this point.
>> +static int stm32_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>> +{
>> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
>> + struct rtc_time *tm = &alrm->time;
>> + unsigned int alrmar, cr, isr;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&rtc->lock, irqflags);
>> +
>> + alrmar = stm32_rtc_readl(rtc, STM32_RTC_ALRMAR);
>> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
>> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
>> +
>> + spin_unlock_irqrestore(&rtc->lock, irqflags);
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_DATE_MASK) {
>> + /*
>> + * Date/day don't care in Alarm comparison so alarm triggers
>
> I guess you meant "doesn't matter" (that is also valid for the other
> usages of "don't care".
>
>> + * every day
>> + */
>> + tm->tm_mday = -1;
>> + tm->tm_wday = -1;
>> + } else {
>> + if (alrmar & STM32_RTC_ALRMXR_WDSEL) {
>> + /* Alarm is set to a day of week */
>> + tm->tm_mday = -1;
>> + tm->tm_wday = (alrmar & STM32_RTC_ALRMXR_WDAY) >>
>> + STM32_RTC_ALRMXR_WDAY_SHIFT;
>> + tm->tm_wday %= 7;
>> + } else {
>> + /* Alarm is set to a day of month */
>> + tm->tm_wday = -1;
>> + tm->tm_mday = (alrmar & STM32_RTC_ALRMXR_DATE) >>
>> + STM32_RTC_ALRMXR_DATE_SHIFT;
>> + }
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_HOUR_MASK) {
>> + /* Hours don't care in Alarm comparison */
>> + tm->tm_hour = -1;
>> + } else {
>> + tm->tm_hour = (alrmar & STM32_RTC_ALRMXR_HOUR) >>
>> + STM32_RTC_ALRMXR_HOUR_SHIFT;
>> + if (alrmar & STM32_RTC_ALRMXR_PM)
>> + tm->tm_hour += 12;
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_MIN_MASK) {
>> + /* Minutes don't care in Alarm comparison */
>> + tm->tm_min = -1;
>> + } else {
>> + tm->tm_min = (alrmar & STM32_RTC_ALRMXR_MIN) >>
>> + STM32_RTC_ALRMXR_MIN_SHIFT;
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_SEC_MASK) {
>> + /* Seconds don't care in Alarm comparison */
>> + tm->tm_sec = -1;
>> + } else {
>> + tm->tm_sec = (alrmar & STM32_RTC_ALRMXR_SEC) >>
>> + STM32_RTC_ALRMXR_SEC_SHIFT;
>> + }
>> +
> I'm not sure those multiple cases (including STM32_RTC_ALRMXR_WDSEL) are
> useful because the core will always give you valid tm_sec, tm_min,
> tm_hour and tm_mday (it is actually checked up to four times!) so you
> should always end up in the same configuration.
>
> If you think some code other than Linux may set an alarm (e.g. the
> bootloader) then you may keep them in read_alarm but at least you can
> remove them from set_alarm.
>
>
Ok I'll clean this part.
>> +static int stm32_rtc_probe(struct platform_device *pdev)
>> +{
>> + struct stm32_rtc *rtc;
>> + struct resource *res;
>> + int ret;
>> +
>> + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
>> + if (!rtc)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + rtc->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(rtc->base))
>> + return PTR_ERR(rtc->base);
>> +
>> + dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "st,syscfg");
>> + if (IS_ERR(dbp)) {
>> + dev_err(&pdev->dev, "no st,syscfg\n");
>> + return PTR_ERR(dbp);
>> + }
>> +
>> + spin_lock_init(&rtc->lock);
>> +
>> + rtc->ck_rtc = devm_clk_get(&pdev->dev, "ck_rtc");
>> + if (IS_ERR(rtc->ck_rtc)) {
>> + dev_err(&pdev->dev, "no ck_rtc clock");
>> + return PTR_ERR(rtc->ck_rtc);
>> + }
>> +
>> + ret = clk_prepare_enable(rtc->ck_rtc);
>> + if (ret)
>> + return ret;
>> +
>> + if (dbp)
>> + regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
>> +
>> + ret = stm32_rtc_init(pdev, rtc);
>> + if (ret)
>> + goto err;
>> +
>
> Isn't that RTC backuped in some way, do you really need to reinit it
> each time the system reboots?
>
>
Indeed, RTC is backuped. I need to reinit it only if RTC parent clock
has changed.
Best Regards,
Amelie
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and usable memory range
From: Neil Armstrong @ 2016-12-12 10:18 UTC (permalink / raw)
To: heinrich.schuchardt, khilman, carlo
Cc: Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel,
devicetree
The Amlogic Meson GXBB secure monitor uses part of the memory space, this
patch adds these reserved zones and redefines the usable memory range for
each boards.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 21 +++++++++++++++++++++
.../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts | 2 +-
.../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 2 +-
.../arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 +-
11 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
index 7a078be..ac40b2d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
@@ -56,7 +56,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_boot: regulator-vddio_boot {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..e085588 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -55,6 +55,27 @@
#address-cells = <2>;
#size-cells = <2>;
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ secos: secos {
+ reg = <0x0 0x05300000 0x0 0x2000000>;
+ no-map;
+ };
+
+ pstore: pstore {
+ reg = <0x0 0x07300000 0x0 0x100000>;
+ no-map;
+ };
+
+ secmon: secmon {
+ reg = <0x0 0x10000000 0x0 0x200000>;
+ no-map;
+ };
+ };
+
cpus {
#address-cells = <0x2>;
#size-cells = <0x0>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 9696820..25b8832 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -62,7 +62,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
leds {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..839c66a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -61,7 +61,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
usb_otg_pwr: regulator-usb-pwrs {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..9a39518 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -55,7 +55,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
usb_pwr: regulator-usb-pwrs {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
index 62fb496..287a4c7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
index 9a9663a..8bdbbe2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
index 2fe167b..2d85295 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
index e99101a..4ec2bbb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
@@ -60,7 +60,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_card: gpio-regulator {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
index 9639f01..b8b5b74 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
@@ -59,7 +59,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index f859d75..1544747 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -62,7 +62,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_boot: regulator-vddio-boot {
--
2.7.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox