* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Thomas Petazzoni @ 2016-11-10 10:15 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87wpgbekwg.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Thu, 10 Nov 2016 10:36:47 +0100, Gregory CLEMENT wrote:
> >> - bootrom {
> >> + bootrom@0 {
> >> compatible = "marvell,bootrom";
> >> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
> >
> > I am still not sure whether this "0" unit address is correct compared
> > to the reg property being passed.
>
> I chose to use the adress register inside the window memory.
Which I think is bogus, as my example below highlighted.
> > A good example of why I'm worried is the sa-sram case:
> >
> > + crypto_sram0: sa-sram0@0 {
> > compatible = "mmio-sram";
> > reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
> >
> > + crypto_sram1: sa-sram1@0 {
> > compatible = "mmio-sram";
> > reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
> >
> > The node names should be just "sram" without a number. Indeed for UARTs
> > for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
> > uart1@ABC. But then, if you do that, with your scheme, you end up with
> > both nodes named sa-sram@0.
> >
> > Which clearly shows that the way you set this unit-address is not
> > correct: those two devices are mapped at completely different
> > locations, but you end up with an identical unit address.
> >
> > I have no idea what is the rule for setting the unit address in this
> > case, but I'm pretty sure the rule you've chosen is not good.
>
> I don't know if there is an existing rules for this case. But I see your
> concern. What I propose then is to expose the memory windows ID by
> adding the target and the attributes like this:
>
> crypto_sram0: sa-sram@09_09_0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>
>
> crypto_sram1: sa-sram@09_05_0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
I have no idea if 09_05_0 is considered a valid unit address. Indeed
the _ character in an address looks a bit weird.
I guess we need guidance from the DT binding maintainers on this, since
it's really a matter of interpreting what "unit address" means in
relation to the value of the "reg" property.
Rob, Mark?
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 5/6] Documentation: bindings: add documentation for ir-spi device driver
From: Andi Shyti @ 2016-11-10 10:02 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Rob Herring, Mauro Carvalho Chehab, Sean Young, Mark Rutland,
Richard Purdie, linux-media, devicetree, linux-leds, linux-kernel
In-Reply-To: <13210179-ea3f-6106-e3c0-fa30b83e23cc@samsung.com>
Hi Jacek,
> > > > > Only DT bindings of LED class drivers should be placed in
> > > > > Documentation/devicetree/bindings/leds. Please move it to the
> > > > > media bindings.
> > > >
> > > > that's where I placed it first, but Rob asked me to put it in the
> > > > LED directory and Cc the LED mailining list.
> > > >
> > > > That's the discussion of the version 2:
> > > >
> > > > https://lkml.org/lkml/2016/9/12/380
> > > >
> > > > Rob, Jacek, could you please agree where I can put the binding?
> > >
> > > I'm not sure if this is a good approach. I've noticed also that
> > > backlight bindings have been moved to leds, whereas they don't look
> > > similarly.
> > >
> > > We have common.txt LED bindings, that all LED class drivers' bindings
> > > have to follow. Neither backlight bindings nor these ones do that,
> > > which introduces some mess.
> >
> > And there are probably LED bindings that don't follow common.txt either.
> >
> > > Eventually adding a sub-directory, e.g. remote_control could make it
> > > somehow logically justified, but still - shouldn't bindings be
> > > placed in the documentation directory related to the subsystem of the
> > > driver they are predestined to?
> >
> > No. While binding directories often mirror the driver directories, they
> > are not the same. Bindings are grouped by types of h/w and IR LEDs are a
> > type of LED.
> >
> > If you prefer a sub-dir, that is fine with me.
>
> Fine. So how about sub-dir "ir" ?
would we put here all the remote control bindings that currently
are under media?
Thanks,
Andi
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Gregory CLEMENT @ 2016-11-10 9:36 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161110092221.0219490b-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Thomas,
On jeu., nov. 10 2016, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hello,
>
> On Thu, 10 Nov 2016 01:09:50 +0100, Gregory CLEMENT wrote:
>
>> - bootrom {
>> + bootrom@0 {
>> compatible = "marvell,bootrom";
>> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
>
> I am still not sure whether this "0" unit address is correct compared
> to the reg property being passed.
I chose to use the adress register inside the window memory.
>
> A good example of why I'm worried is the sa-sram case:
>
> + crypto_sram0: sa-sram0@0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>
> + crypto_sram1: sa-sram1@0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
>
> The node names should be just "sram" without a number. Indeed for UARTs
> for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
> uart1@ABC. But then, if you do that, with your scheme, you end up with
> both nodes named sa-sram@0.
>
> Which clearly shows that the way you set this unit-address is not
> correct: those two devices are mapped at completely different
> locations, but you end up with an identical unit address.
>
> I have no idea what is the rule for setting the unit address in this
> case, but I'm pretty sure the rule you've chosen is not good.
I don't know if there is an existing rules for this case. But I see your
concern. What I propose then is to expose the memory windows ID by
adding the target and the attributes like this:
crypto_sram0: sa-sram@09_09_0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
crypto_sram1: sa-sram@09_05_0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
Gregory
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [v16, 0/7] Fix eSDHC host version register bug
From: Geert Uytterhoeven @ 2016-11-10 9:26 UTC (permalink / raw)
To: Ulf Hansson
Cc: Yangbo Lu, linux-mmc, Scott Wood, Arnd Bergmann,
linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-clk, iommu,
netdev@vger.kernel.org, Greg Kroah-Hartman, Mark Rutland,
Rob Herring, Russell King, Jochen Friedrich, Joerg Roedel,
Claudiu
In-Reply-To: <CAPDyKFrcAN_pqgtGaUanfB2zh97zGcL23m5VDtJ3g==NJeRrfA@mail.gmail.com>
Hi Ulf,
On Wed, Nov 9, 2016 at 7:27 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 9 November 2016 at 04:14, Yangbo Lu <yangbo.lu@nxp.com> wrote:
>> This patchset is used to fix a host version register bug in the T4240-R1.0-R2.0
>> eSDHC controller. To match the SoC version and revision, 15 previous version
>> patchsets had tried many methods but all of them were rejected by reviewers.
>> Such as
>> - dts compatible method
>> - syscon method
>> - ifdef PPC method
>> - GUTS driver getting SVR method
>> Anrd suggested a soc_device_match method in v10, and this is the only available
>> method left now. This v11 patchset introduces the soc_device_match interface in
>> soc driver.
>>
>> The first four patches of Yangbo are to add the GUTS driver. This is used to
>> register a soc device which contain soc version and revision information.
>> The other three patches introduce the soc_device_match method in soc driver
>> and apply it on esdhc driver to fix this bug.
>>
>> ---
>> Changes for v15:
>> - Dropped patch 'dt: bindings: update Freescale DCFG compatible'
>> since the work had been done by below patch on ShawnGuo's linux tree.
>> 'dt-bindings: fsl: add LS1043A/LS1046A/LS2080A compatible for SCFG
>> and DCFG'
>> - Fixed error code issue in guts driver
>> Changes for v16:
>> - Dropped patch 'powerpc/fsl: move mpc85xx.h to include/linux/fsl'
>> - Added a bug-fix patch from Geert
>> ---
>>
>> Arnd Bergmann (1):
>> base: soc: introduce soc_device_match() interface
>>
>> Geert Uytterhoeven (1):
>> base: soc: Check for NULL SoC device attributes
> Thanks, applied on my mmc tree for next!
Oops, the above two commits (plus two more enhancements) are also a dependency
for Samsung and Renesas. Hence the plan was to use an immutable branch for
that...
Ulf, would it still be possible to replace these two commits in mmc/next:
8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
6fa350172b098f0f base: soc: Check for NULL SoC device attributes
by a merge of the immutable branch I've just created?
Cfr, the other thread "[PATCH v2 0/7] soc: renesas: Identify SoC and register
with the SoC bus".
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 5/5] media: platform: rcar_drif: Add DRIF support
From: Laurent Pinchart @ 2016-11-10 9:22 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: robh+dt, mark.rutland, mchehab, hverkuil, sakari.ailus, crope,
chris.paterson2, geert+renesas, linux-media, devicetree,
linux-renesas-soc
In-Reply-To: <1478706284-59134-6-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>
Hi Ramesh,
Thank you for the patch.
On Wednesday 09 Nov 2016 15:44:44 Ramesh Shanmugasundaram wrote:
> This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3 SoCs.
> The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF
> device represents a channel and each channel can have one or two
> sub-channels respectively depending on the target board.
>
> DRIF supports only Rx functionality. It receives samples from a RF
> frontend tuner chip it is interfaced with. The combination of DRIF and the
> tuner device, which is registered as a sub-device, determines the receive
> sample rate and format.
>
> In order to be compliant as a V4L2 SDR device, DRIF needs to bind with
> the tuner device, which can be provided by a third party vendor. DRIF acts
> as a slave device and the tuner device acts as a master transmitting the
> samples. The driver allows asynchronous binding of a tuner device that
> is registered as a v4l2 sub-device. The driver can learn about the tuner
> it is interfaced with based on port endpoint properties of the device in
> device tree. The V4L2 SDR device inherits the controls exposed by the
> tuner device.
>
> The device can also be configured to use either one or both of the data
> pins at runtime based on the master (tuner) configuration.
>
> Signed-off-by: Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> ---
> .../devicetree/bindings/media/renesas,drif.txt | 136 ++
> drivers/media/platform/Kconfig | 25 +
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/rcar_drif.c | 1574
> ++++++++++++++++++++ 4 files changed, 1736 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
> create mode 100644 drivers/media/platform/rcar_drif.c
>
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file mode
> 100644
> index 0000000..d65368a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,136 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +------------------------------------------------------------
> +
> +R-Car Gen3 DRIF is a serial slave device. It interfaces with a master
> +device as shown below
> +
> ++---------------------+ +---------------------+
> +| |-----SCK------->|CLK |
> +| Master |-----SS-------->|SYNC DRIFn (slave) |
> +| |-----SD0------->|D0 |
> +| |-----SD1------->|D1 |
> ++---------------------+ +---------------------+
> +
> +Each DRIF channel (drifn) consists of two sub-channels (drifn0 & drifn1).
> +The sub-channels are like two individual channels in itself that share the
> +common CLK & SYNC. Each sub-channel has it's own dedicated resources like
> +irq, dma channels, address space & clock.
> +
> +The device tree model represents the channel and each of it's sub-channel
> +as a separate node. The parent channel ties the sub-channels together with
> +their phandles.
> +
> +Required properties of a sub-channel:
> +-------------------------------------
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
> R8A7795 SoC.
> + "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
> device.
> + When compatible with the generic version, nodes must list the
> + SoC-specific version corresponding to the platform first
> + followed by the generic version.
> +- reg: offset and length of that sub-channel.
> +- interrupts: associated with that sub-channel.
> +- clocks: phandle and clock specifier of that sub-channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channel of that sub-channel.
> +- dma-names: names of the DMA channel: "rx".
> +
> +Optional properties of a sub-channel:
> +-------------------------------------
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of a channel:
> +---------------------------------
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- sub-channels : phandles to the two sub-channels.
> +
> +Optional properties of a channel:
> +---------------------------------
> +- port: child port node of a channel that defines the local and remote
> + endpoints. The remote endpoint is assumed to be a tuner subdevice
> + endpoint.
> +- renesas,syncmd : sync mode
> + 0 (Frame start sync pulse mode. 1-bit width pulse
> + indicates start of a frame)
> + 1 (L/R sync or I2S mode) (default)
> +- renesas,lsb-first : empty property indicates lsb bit is received
> first.
> + When not defined msb bit is received first (default)
Shouldn't those two properties be instead queried from the tuner at runtime
through the V4L2 subdev API ?
> +- renesas,syncac-pol-high : empty property indicates sync signal polarity.
> + When defined, active high or high->low sync signal.
> + When not defined, active low or low->high sync signal
> + (default)
This could be queried too, except that an inverter could be present on the
board, so it has to be specified in DT. I would however try to standardize it
the same way that hsync-active and vsync-active are standardized in
Documentation/devicetree/bindings/media/video-interfaces.txt.
> +- renesas,dtdl : delay between sync signal and start of reception.
Are this and the next property meant to account for PCB traces delays ?
> + Must contain one of the following values:
> + 0 (no bit delay)
> + 50 (0.5-clock-cycle delay)
> + 100 (1-clock-cycle delay) (default)
> + 150 (1.5-clock-cycle delay)
> + 200 (2-clock-cycle delay)
How about specifying the property in half clock cycle units, from 0 to 4 ?
> +- renesas,syncdl : delay between end of reception and sync signal
> edge.
> + Must contain one of the following values:
> + 0 (no bit delay) (default)
> + 50 (0.5-clock-cycle delay)
> + 100 (1-clock-cycle delay)
> + 150 (1.5-clock-cycle delay)
> + 200 (2-clock-cycle delay)
> + 300 (3-clock-cycle delay)
> +
> +Example
> +--------
> +
> +SoC common dtsi file
> +
> + drif00: rif@e6f40000 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + reg = <0 0xe6f40000 0 0x64>;
> + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 515>;
> + clock-names = "fck";
> + dmas = <&dmac1 0x20>, <&dmac2 0x20>;
> + dma-names = "rx", "rx";
> + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> + status = "disabled";
> + };
> +
> + drif01: rif@e6f50000 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + reg = <0 0xe6f50000 0 0x64>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 514>;
> + clock-names = "fck";
> + dmas = <&dmac1 0x22>, <&dmac2 0x22>;
> + dma-names = "rx", "rx";
> + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> + status = "disabled";
> + };
> +
> + drif0: rif@0 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + sub-channels = <&drif00>, <&drif01>;
> + status = "disabled";
> + };
I'm afraid this really hurts my eyes, especially using the same compatible
string for both the channel and sub-channel nodes.
We need to decide how to model the hardware in DT. Given that the two channels
are mostly independent having separate DT nodes makes sense to me. However, as
they share the clock and sync signals, somehow grouping them makes sense. I
see three ways to do so, and there might be more.
1. Adding an extra DT node for the channels group, with phandles to the two
channels. This is what you're proposing here.
2. Adding an extra DT node for the channels group, as a parent of the two
channels.
3. Adding phandles to the channels, pointing to each other, or possibly a
phandle from channel 0 pointing to channel 1.
Neither of these options seem perfect to me. I don't like option 1 as the
group DT node really doesn't describe a hardware block. If we want to use a DT
node to convey group information, option 2 seems better to me. However, it
somehow abuses the DT parent-child model that is supposed to describe
relationships from a control bus point of view. Option 3 has the drawback of
not scaling properly, at least with phandles in both channels pointing to the
other one.
Rob, Geert, tell me you have a fourth idea I haven't thought of that would
solve all those problems :-)
> +Board specific dts file
> +
> +&drif00 {
> + status = "okay";
> +};
> +
> +&drif01 {
> + status = "okay";
> +};
> +
> +&drif0 {
> + pinctrl-0 = <&drif0_pins>;
> + pinctrl-names = "default";
> + renesas,syncac-pol-high;
> + status = "okay";
> + port {
> + drif0_ep: endpoint {
> + remote-endpoint = <&tuner_subdev_ep>;
> + };
> + };
> +};
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Geert Uytterhoeven @ 2016-11-10 9:22 UTC (permalink / raw)
To: Ulf Hansson
Cc: Mark Rutland, devicetree@vger.kernel.org, Dirk Behme,
Arnd Bergmann, Simon Horman, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, Linux-Renesas,
linux-samsung-soc@vger.kernel.org, Rob Herring, Yangbo Lu,
linuxppc-dev@lists.ozlabs.org, Pankaj Dubey,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <6723536.97ljqcB8tS@wuerfel>
Hi Ulf,
On Wed, Nov 9, 2016 at 10:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, November 9, 2016 6:19:06 PM CET Geert Uytterhoeven wrote:
>> On Wed, Nov 9, 2016 at 5:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
>> >> > And Samsung.
>> >> > Shall I create the immutable branch now?
>> >>
>> >> Arnd: are you happy with the new patches and changes?
>> >
>> > I still had some comments for patch 7, but that shouldn't stop
>> > you from creating a branch for the first three so everyone can
>> > build on top of that.
>>
>> Thanks!
>>
>> What about patch [4/7]?
>> Haven't you received it? Your address was in the To-line for all 7 patches.
>
> Ok, I see it now, looks good. That should be included as well then.
Thanks, I've created the branch/tag :
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
branch soc-device-match
signed tag soc-device-match-tag1
In the mean time, Ulf has applied the first two patches to mmc/next, on top
of lots of MMC work :-(
Ulf, as this is not only a dependency for Freescale/NXP (for sdhci-of-esdhc),
but also for Samsung and Renesas, would it still be possible to replace these
two commits
8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
6fa350172b098f0f base: soc: Check for NULL SoC device attributes
by a merge of soc-device-match-tag1?
You can find more info in the full thread at
https://www.spinics.net/lists/devicetree/msg148558.html
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] phy: rockchip-inno-usb2: correct 480MHz output clock stable time
From: Heiko Stübner @ 2016-11-10 9:21 UTC (permalink / raw)
To: wlf
Cc: Doug Anderson, Kishon Vijay Abraham I,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
open list:ARM/Rockchip SoC..., devicetree@vger.kernel.org,
Rob Herring, Frank Wang, 黄涛, Brian Norris,
Guenter Roeck, Matthias Kaehlcke, linux-clk
In-Reply-To: <9eeb3d84-f850-4b76-ce6a-67cc92d6de3b@rock-chips.com>
Am Donnerstag, 10. November 2016, 10:54:49 schrieb wlf:
> Hi Doug,
>
> 在 2016年11月10日 04:54, Doug Anderson 写道:
> > Hi,
> >
> > On Mon, Nov 7, 2016 at 5:00 AM, William Wu <wulf@rock-chips.com> wrote:
> >> We found that the system crashed due to 480MHz output clock of
> >> USB2 PHY was unstable after clock had been enabled by gpu module.
> >>
> >> Theoretically, 1 millisecond is a critical value for 480MHz
> >> output clock stable time, so we try to change the delay time
> >> to 1.2 millisecond to avoid this issue.
> >>
> >> Signed-off-by: William Wu <wulf@rock-chips.com>
> >> ---
> >>
> >> drivers/phy/phy-rockchip-inno-usb2.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
> >> b/drivers/phy/phy-rockchip-inno-usb2.c index ecfd7d1..8f2d2b6 100644
> >> --- a/drivers/phy/phy-rockchip-inno-usb2.c
> >> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> >> @@ -267,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct
> >> clk_hw *hw)>>
> >> return ret;
> >>
> >> /* waitting for the clk become stable */
> >>
> >> - mdelay(1);
> >> + udelay(1200);
> >
> > Several people who have seen this patch have expressed concern that a
> > 1.2 ms delay is pretty long for something that's supposed to be
> > "atomic" like a clk_enable(). Consider that someone might call
> > clk_enable() while interrupts are disabled and that a 1.2 ms interrupt
> > latency is not so great.
> >
> > It seems like this clock should be moved to be enabled in "prepare"
> > and the "enable" should be a no-op. This is a functionality change,
> > but I don't think there are any real users for this clock at the
> > moment so it should be fine.
> >
> > (of course, the 1 ms latency that existed before this patch was still
> > pretty bad, but ...)
>
> Thanks a lot for your suggestion.
> I agree with you. clk_enable() will call spin_lock_irqsave() to disable
> interrupt, and we add
> more than 1ms in clk_enable may cause big latency.
>
> And according to clk_prepare() description:
> In a simple case, clk_prepare can be used instead of clk_enable to
> ungate a clk if the
> operation may sleep. One example is a clk which is accessed over I2c.
>
> So maybe we can remove the clock to clk_prepare.
>
> Hi Heiko, Frank,
> What do you think of it?
moving to clk_prepare sounds sensible. That way you can switch from delay to
sleep functions as well.
Heiko
^ permalink raw reply
* Re: [PATCH 2/3] drm/bridge: Add ti-ftp410 HDMI transmitter driver
From: Jyri Sarha @ 2016-11-10 9:16 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, bcousson, khilman, dri-devel, bgolaszewski,
tomi.valkeinen
In-Reply-To: <12424154.kJqhEf2pOz@avalon>
On 11/03/16 19:46, Laurent Pinchart wrote:
> Hi Jyri,
>
> Thank you for the patch.
>
> On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote:
>> Add very basic ti-ftp410 HDMI transmitter driver. The only feature
>> separating this from a completely dummy bridge is the DDC i2c
>> support. However, other HW specific features may be added later when
>> needed. For instance there is a set of registers behind i2c if it is
>> connected. The implementations is tested against my new tilcdc bridge
>> support and works with BeagleBone DVI-D Cape Rev A3. A DT binding
>> document is also added.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>> .../bindings/display/bridge/ti,tfp410.txt | 30 ++++
>> drivers/gpu/drm/bridge/Kconfig | 7 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/ti-tfp410.c | 199 ++++++++++++++++++
>> 4 files changed, 237 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt create mode
>> 100644 drivers/gpu/drm/bridge/ti-tfp410.c
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt new file
>> mode 100644
>> index 0000000..dc93713
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> @@ -0,0 +1,30 @@
>> +TFP410 HDMI/DVI bridge bindings
>
> I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell
> whether the device is a receiver or transmitter.
>
>> +Required properties:
>> + - compatible: "ti,tfp410"
>
> The device is an I2C slave, it should have a reg property. Given that the chip
> can be used without being controlled through I2C, the reg property should be
> optional. You should document this clearly, and explain how the DT node can be
> instantiated as a child of an I2C controller when the I2C interface is used,
> or in other parts of the device tree otherwise.
>
>> +Optional properties:
>> + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing
>
> The TFP410 doesn't handle DDC, this property should be part of the connector
> node.
>
>> +Optional subnodes:
>> + - video input: this subnode can contain a video input port node
>> + to connect the bridge to a display controller output (See this
>> + documentation [1]).
>
> You also need an output port for the DVI output. Those two ports should be
> required, not optional.
>
Ok. So I need another device node. Should I create some specific
compatible string for connectors behind tfp410, or a generic DVI/HDMI
connector with optional ddc-i2c phandle?
The implementation side is not so critical, because it more easily
changed, but should I create an independent generic platform-device
driver for such DVI/HDMI connector or just implement the connector side
within tfp410 driver?
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +Example:
>> + hdmi-bridge {
>> + compatible = "ti,tfp410";
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + bridge_in: endpoint {
>> + remote-endpoint = <&dc_out>;
>> + };
>> + };
>> + };
>> + };
>
>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index bd6acc8..a424e03 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
>> ---help---
>> Toshiba TC358767 eDP bridge chip driver.
>>
>> +config DRM_TI_TFP410
>> + tristate "TI TFP410 DVI/HDMI bridge"
>> + depends on OF
>> + select DRM_KMS_HELPER
>> + ---help---
>> + Texas Instruments TFP410 DVI/HDMI Transmitter driver
>> +
>> source "drivers/gpu/drm/bridge/analogix/Kconfig"
>>
>> source "drivers/gpu/drm/bridge/adv7511/Kconfig"
>> diff --git a/drivers/gpu/drm/bridge/Makefile
>> b/drivers/gpu/drm/bridge/Makefile index 97ed1a5..8b065d9 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
>> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> +obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
>> b/drivers/gpu/drm/bridge/ti-tfp410.c new file mode 100644
>> index 0000000..b0753d2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
>> @@ -0,0 +1,199 @@
>> +/*
>> + * Copyright (C) 2016 Texas Instruments
>> + * Author: Jyri Sarha <jsarha@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> by + * the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_graph.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +struct tfp410 {
>> + struct drm_bridge bridge;
>> + struct drm_connector connector;
>> +
>> + struct i2c_adapter *ddc;
>> +
>> + struct device *dev;
>> +};
>> +
>> +static inline struct tfp410 *
>> +drm_bridge_to_tfp410(struct drm_bridge *bridge)
>> +{
>> + return container_of(bridge, struct tfp410, bridge);
>> +}
>> +
>> +static inline struct tfp410 *
>> +drm_connector_to_tfp410(struct drm_connector *connector)
>> +{
>> + return container_of(connector, struct tfp410, connector);
>> +}
>> +
>> +static int tfp410_get_modes(struct drm_connector *connector)
>> +{
>> + struct tfp410 *hdmi = drm_connector_to_tfp410(connector);
>> + struct edid *edid;
>> + int ret;
>> +
>> + if (!hdmi->ddc)
>> + goto fallback;
>> +
>> + edid = drm_get_edid(connector, hdmi->ddc);
>> + if (!edid) {
>> + DRM_INFO("EDID read failed. Fallback to standard modes\n");
>> + goto fallback;
>> + }
>> +
>> + drm_mode_connector_update_edid_property(connector, edid);
>> +
>> + return drm_add_edid_modes(connector, edid);
>> +fallback:
>> + /* No EDID, fallback on the XGA standard modes */
>> + ret = drm_add_modes_noedid(connector, 1920, 1200);
>> +
>> + /* And prefer a mode pretty much anything can handle */
>> + drm_set_preferred_mode(connector, 1024, 768);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
>> + .get_modes = tfp410_get_modes,
>> +};
>> +
>> +static enum drm_connector_status
>> +tfp410_connector_detect(struct drm_connector *connector, bool force)
>> +{
>> + struct tfp410 *hdmi = drm_connector_to_tfp410(connector);
>> +
>> + if (hdmi->ddc) {
>> + if (drm_probe_ddc(hdmi->ddc))
>> + return connector_status_connected;
>> + else
>> + return connector_status_disconnected;
>> + }
>> +
>> + return connector_status_unknown;
>> +}
>> +
>> +static const struct drm_connector_funcs tfp410_con_funcs = {
>> + .dpms = drm_atomic_helper_connector_dpms,
>> + .detect = tfp410_connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = drm_connector_cleanup,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static int tfp410_attach(struct drm_bridge *bridge)
>> +{
>> + struct tfp410 *hdmi = drm_bridge_to_tfp410(bridge);
>> + int ret;
>> +
>> + if (!bridge->encoder) {
>> + dev_err(hdmi->dev, "Missing encoder\n");
>> + return -ENODEV;
>> + }
>> +
>> + drm_connector_helper_add(&hdmi->connector,
>> + &tfp410_con_helper_funcs);
>> + ret = drm_connector_init(bridge->dev, &hdmi->connector,
>> + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
>> + if (ret) {
>> + dev_err(hdmi->dev, "drm_connector_init() failed: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drm_mode_connector_attach_encoder(&hdmi->connector,
>> + bridge->encoder);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
>> + .attach = tfp410_attach,
>> +};
>> +
>> +static int tfp410_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *ddc_phandle;
>> + struct tfp410 *hdmi;
>> + int ret;
>> +
>> + if (!pdev->dev.of_node) {
>> + dev_err(&pdev->dev, "device-tree data is missing\n");
>> + return -ENXIO;
>> + }
>> +
>> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
>> + if (!hdmi)
>> + return -ENOMEM;
>> + platform_set_drvdata(pdev, hdmi);
>> +
>> + ddc_phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c", 0);
>> + if (ddc_phandle) {
>> + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
>> + of_node_put(ddc_phandle);
>> + if (!hdmi->ddc)
>> + return -EPROBE_DEFER;
>> + } else {
>> + dev_info(&pdev->dev,
>> + "No ddc i2c bus, disabling EDID readout\n");
>> + }
>> +
>> + hdmi->bridge.funcs = &tfp410_bridge_funcs;
>> + hdmi->bridge.of_node = pdev->dev.of_node;
>> + hdmi->dev = &pdev->dev;
>> +
>> + ret = drm_bridge_add(&hdmi->bridge);
>> + if (ret) {
>> + dev_err(&pdev->dev, "drm_bridge_add() failed: %d\n", ret);
>> + goto fail;
>> + }
>> +
>> + return 0;
>> +fail:
>> + i2c_put_adapter(hdmi->ddc);
>> + return ret;
>> +}
>> +
>> +static int tfp410_remove(struct platform_device *pdev)
>> +{
>> + struct tfp410 *hdmi = platform_get_drvdata(pdev);
>> +
>> + drm_bridge_remove(&hdmi->bridge);
>> +
>> + if (hdmi->ddc)
>> + i2c_put_adapter(hdmi->ddc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id tfp410_match[] = {
>> + { .compatible = "ti,tfp410" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, tfp410_match);
>> +
>> +struct platform_driver tfp410_driver = {
>> + .probe = tfp410_probe,
>> + .remove = tfp410_remove,
>> + .driver = {
>> + .name = "tfp410-bridge",
>> + .of_match_table = tfp410_match,
>> + },
>> +};
>> +module_platform_driver(tfp410_driver);
>> +
>> +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
>> +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver");
>> +MODULE_LICENSE("GPL");
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-10 9:12 UTC (permalink / raw)
To: linux-arm-kernel
Cc: zhichang.yuan, mark.rutland@arm.com, devicetree@vger.kernel.org,
lorenzo.pieralisi@arm.com, Gabriele Paoloni, minyard@acm.org,
linux-pci@vger.kernel.org, benh@kernel.crashing.org, John Garry,
will.deacon@arm.com, linux-kernel@vger.kernel.org, xuwei (O),
Linuxarm, zourongrong@gmail.com, robh+dt@kernel.org,
kantyzc@163.com, linux-serial
In-Reply-To: <5824165A.4040303@hisilicon.com>
On Thursday, November 10, 2016 2:40:26 PM CET zhichang.yuan wrote:
> On 2016/11/10 5:34, Arnd Bergmann wrote:
> > On Wednesday, November 9, 2016 12:10:43 PM CET Gabriele Paoloni wrote:
> >>> On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote:
> >>>> + /*
> >>>> + * The first PCIBIOS_MIN_IO is reserved specifically for
> >>> indirectIO.
> >>>> + * It will separate indirectIO range from pci host bridge to
> >>>> + * avoid the possible PIO conflict.
> >>>> + * Set the indirectIO range directly here.
> >>>> + */
> >>>> + lpcdev->io_ops.start = 0;
> >>>> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1;
> >>>> + lpcdev->io_ops.devpara = lpcdev;
> >>>> + lpcdev->io_ops.pfin = hisilpc_comm_in;
> >>>> + lpcdev->io_ops.pfout = hisilpc_comm_out;
> >>>> + lpcdev->io_ops.pfins = hisilpc_comm_ins;
> >>>> + lpcdev->io_ops.pfouts = hisilpc_comm_outs;
> >>>
> >>> I have to look at patch 2 in more detail again, after missing a few
> >>> review
> >>> rounds. I'm still a bit skeptical about hardcoding a logical I/O port
> >>> range here, and would hope that we can just go through the same
> >>> assignment of logical port ranges that we have for PCI buses,
> >>> decoupling
> >>> the bus addresses from the linux-internal ones.
> >>
> >> The point here is that we want to avoid any conflict/overlap between
> >> the LPC I/O space and the PCI I/O space. With the assignment above
> >> we make sure that LPC never interfere with PCI I/O space.
> >
> > But we already abstract the PCI I/O space using dynamic registration.
> > There is no need to hardcode the logical address for ISA, though
> > I think we can hardcode the bus address to start at zero here.
>
> Do you means that we can pick up the maximal I/O address from all children's
> device resources??
The driver should not look at the resources of its children, just
register a range of addresses dynamically, as I suggested in an
earlier review.
Your current version has
if (arm64_extio_ops->pfout) \
arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
addr, value, sizeof(type)); \
Instead, just subtract the start of the range from the logical
port number to transform it back into a bus-local port number:
if (arm64_extio_ops->pfout) \
arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
addr - arm64_extio_ops->start, value, sizeof(type)); \
We know that the ISA/LPC bus can only have up to 65536 ports,
so you can register all of those, or possibly limit it further to
1024 or 4096 ports, whichever matches the bus implementation.
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets
From: Vivek Gautam @ 2016-11-10 9:03 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: robh+dt, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ca4054a8-a813-0f50-3224-1e418eaa7095-l0cyMroinI0@public.gmane.org>
Hi Kishon,
On Thu, Oct 27, 2016 at 1:41 AM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Wednesday 19 October 2016 04:13 PM, Vivek Gautam wrote:
>> Qualcomm SOCs have QMP phy controller that provides support
>> to a number of controller, viz. PCIe, UFS, and USB.
>> Add a new driver, based on generic phy framework, for this
>> phy controller.
>>
>> USB3-phy changes: Based on phy-msm-ssusb-qmp driver available on
>> msm-4.4 kernel @codeaurora[1].
>> PCIe-phy changes: Based on msm8996-pcie-phy driver posted by
>> Srinivas [2].
>>
>> [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/log/?h=caf/3.18/msm-3.18
>> [2] https://patchwork.kernel.org/patch/9318947/
>
> use only lkml links here.
Ok.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Cc: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>> ---
>> .../devicetree/bindings/phy/qcom-qmp-phy.txt | 61 ++
>> drivers/phy/Kconfig | 8 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-qcom-qmp.c | 1154 ++++++++++++++++++++
>> 4 files changed, 1224 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> create mode 100644 drivers/phy/phy-qcom-qmp.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> new file mode 100644
>> index 0000000..90214aa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> @@ -0,0 +1,61 @@
>> +Qualcomm QMP PHY
>> +----------------
>> +
>> +QMP phy controller supports physical layer functionality for a number of
>> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
>> +
>> +Required properties:
>> + - compatible: compatible list, contains:
>> + "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
>> + "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
>> + - reg: offset and length of the PHY register set.
>> + - #phy-cells: must be 1
>> + - Cell after phy phandle should be the port (lane) number.
>> + - clocks: a list of phandles and clock-specifier pairs,
>> + one for each entry in clock-names.
>> + - clock-names: must be "cfg_ahb" for phy config clock,
>> + "aux" for phy aux clock,
>> + "ref_clk" for 19.2 MHz ref clk,
>> + "ref_clk_src" for reference clock source,
>> + "pipe<port-number>" for pipe clock specific to
>> + each port/lane (Optional).
>> + - resets: a list of phandles and reset controller specifier pairs,
>> + one for each entry in reset-names.
>> + - reset-names: must be "phy" for reset of phy block,
>> + "common" for phy common block reset,
>> + "cfg" for phy's ahb cfg block reset (Optional).
>> + "port<port-number>" for reset specific to
>> + each port/lane. (Optional)
>> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
>> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
>> +
>> +Optional properties:
>> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
>> + pll block.
>> +
>> +Example:
>> + pcie_phy: pciephy@34000 {
>> + compatible = "qcom,qmp-14nm-pcie-phy";
>> + reg = <0x034000 0x3fff>;
>> + #phy-cells = <1>;
>> +
>> + clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>> + <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>> + <&gcc GCC_PCIE_0_PIPE_CLK>,
>> + <&gcc GCC_PCIE_1_PIPE_CLK>,
>> + <&gcc GCC_PCIE_2_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb",
>> + "pipe0", "pipe1", "pipe2";
>> +
>> + vdda-phy-supply = <&pm8994_l28>;
>> + vdda-pll-supply = <&pm8994_l12>;
>> +
>> + resets = <&gcc GCC_PCIE_PHY_BCR>,
>> + <&gcc GCC_PCIE_PHY_COM_BCR>,
>> + <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
>> + <&gcc GCC_PCIE_0_PHY_BCR>,
>> + <&gcc GCC_PCIE_1_PHY_BCR>,
>> + <&gcc GCC_PCIE_2_PHY_BCR>;
>> + reset-names = "phy", "common", "cfg",
>> + "lane0", "lane1", "lane2";
>> + };
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 5547984..d5e2b50f 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -446,6 +446,14 @@ config PHY_STIH41X_USB
>> Enable this to support the USB transceiver that is part of
>> STMicroelectronics STiH41x SoC series.
>>
>> +config PHY_QCOM_QMP
>> + tristate "Qualcomm QMP PHY Driver"
>> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the QMP PHY transceiver that is used
>> + with controllers such as PCIe, UFS, and USB on Qualcomm chips.
>> +
>> config PHY_QCOM_QUSB2
>> tristate "Qualcomm QUSB2 PHY Driver"
>> depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 848489d..fde9fba 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
>> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
>> obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
>> obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
>> +obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
>> obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
>> diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
>> new file mode 100644
>> index 0000000..7e89179
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qmp.c
>> @@ -0,0 +1,1154 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
[snip]
>> +
>> +static struct qmp_phy_init_tbl usb3phy_serdes_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x14),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x08),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x30),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x04),
>> + /* PLL and Loop filter settings */
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x15),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x34),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_CFG, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x0a),
>> + /* SSC settings */
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0xde),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x07),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_tx_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_rx_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x02),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4c),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xbb),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x18),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_pcs_init_tbl[] = {
>> + /* FLL settings */
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL2, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL1, 0x02),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_L, 0x09),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_H_TOL, 0x42),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_MAN_CODE, 0x85),
>> +
>> + /* Lock Det settings */
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG1, 0xd1),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG2, 0x1f),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG3, 0x47),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG2, 0x08),
>> +};
>
> if you can implement the dt mechanism I mentioned in your other patch, all
> these tables can be got rid of and the code to initialize these can also be
> removed.
These are lot of configurations for PLL, TX, RX and PCS. I think it's
hard to take
out common bindings that can serve purpose of other PHYs as well.
[snip]
>> +unsigned int msm8996_pciephy_tx_offsets[] = { 0x1000, 0x2000, 0x3000 };
>
> you can have a separate reg map for each lane and all these can come from dt.
The idea is to avoid the any child nodes for lanes. So, we have the complete
ioremaped region and these offsets to tx, rx and pcs blocks.
>> +unsigned int msm8996_pciephy_rx_offsets[] = { 0x1200, 0x2200, 0x3200 };
>> +unsigned int msm8996_pciephy_pcs_offsets[] = { 0x1400, 0x2400, 0x3400 };
[snip]
>> +static int qcom_qmp_phy_poweron(struct phy *phy)
>> +{
>> + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> + struct qcom_qmp_phy *qphy = phydesc->qphy;
>> + int ret;
>> +
>> + dev_info(&phy->dev, "Powering on QMP phy\n");
>> +
>> + ret = regulator_enable(qphy->vdda_phy);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-phy enable failed, err=%d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_pll);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-pll enable failed, err=%d\n",
>> + __func__, ret);
>> + goto err_vdda_pll;
>> + }
>> +
>> + if (qphy->vddp_ref_clk) {
>> + ret = regulator_enable(qphy->vddp_ref_clk);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-ref-clk enable failed, err=%d\n",
>> + __func__, ret);
>> + goto err_vddp_refclk;
>> + }
>> + }
>> +
>> + if (!qphy->clk_enabled) {
>
> lot of my comments on the previous PHY driver is applicable here. For example
> the clk_enabled is not required.
Sure, will remove this and take care of other things similar to
qusb2 phy driver.
>> + clk_prepare_enable(qphy->ref_clk_src);
>> + clk_prepare_enable(qphy->ref_clk);
>> + clk_prepare_enable(phydesc->pipe_clk);
>> + qphy->clk_enabled = true;
>> + }
[snip]
>> +static struct phy *qcom_qmp_phy_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
>> + int i;
>> +
>> + if (WARN_ON(args->args[0] >= qphy->cfg->nlanes))
>> + return ERR_PTR(-ENODEV);
>> +
>> + for (i = 0; i < qphy->cfg->nlanes; i++) {
>> + if (qphy->phys[i]->index == args->args[0])
>> + break;
>
> finding a PHY based on index is not required. Just have a different label for
> each sub-node and using this label in the controller node should be enough.
Like i said in my comment above, the idea is to avoid any kind of sub-nodes
(child nodes) of the PHY device node. So each lane is registered at
separate indices.
Regards
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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: [RFC 1/5] media: i2c: max2175: Add MAX2175 support
From: Laurent Pinchart @ 2016-11-10 8:46 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: robh+dt@kernel.org, mark.rutland@arm.com, mchehab@kernel.org,
hverkuil@xs4all.nl, sakari.ailus@linux.intel.com, crope@iki.fi,
Chris Paterson, geert@linux-m68k.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <SG2PR06MB1038BC62A3011C8EAB20B61FC3D40@SG2PR06MB1038.apcprd06.prod.outlook.com>
Hi Ramesh,
On Friday 21 Oct 2016 14:49:30 Ramesh Shanmugasundaram wrote:
> > On Wednesday 12 Oct 2016 15:10:25 Ramesh Shanmugasundaram wrote:
> >> This patch adds driver support for MAX2175 chip. This is Maxim
> >> Integrated's RF to Bits tuner front end chip designed for
> >> software-defined radio solutions. This driver exposes the tuner as a
> >> sub-device instance with standard and custom controls to configure the
> >> device.
> >>
> >> Signed-off-by: Ramesh Shanmugasundaram
> >> <ramesh.shanmugasundaram@bp.renesas.com> ---
> >>
> >> .../devicetree/bindings/media/i2c/max2175.txt | 60 +
> >> drivers/media/i2c/Kconfig | 4 +
> >> drivers/media/i2c/Makefile | 2 +
> >> drivers/media/i2c/max2175/Kconfig | 8 +
> >> drivers/media/i2c/max2175/Makefile | 4 +
> >> drivers/media/i2c/max2175/max2175.c | 1624 +++++++++++++
> >> drivers/media/i2c/max2175/max2175.h | 124 ++
> >> 7 files changed, 1826 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> create mode 100644 drivers/media/i2c/max2175/Kconfig create mode
> >> 100644 drivers/media/i2c/max2175/Makefile
> >> create mode 100644 drivers/media/i2c/max2175/max2175.c
> >> create mode 100644 drivers/media/i2c/max2175/max2175.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file
> >> mode 100644
> >> index 0000000..2250d5f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> @@ -0,0 +1,60 @@
[snip]
> >> +Optional properties:
> >> +--------------------
> >> +- maxim,slave : empty property indicates this is a slave of
> >> another
> >> + master tuner. This is used to define two tuners in
> >> + diversity mode (1 master, 1 slave). By default each
> >> + tuner is an individual master.
> >
> > Would it be useful to make that property a phandle to the master tuner, to
> > give drivers a way to access the master ? I haven't checked whether there
> > could be use cases for that.
>
> As of now, I cannot find any use case for it from the datasheet. In future,
> we could add if such need arise.
My point is that making the maxim,slave property a phandle now would allow
handling such future cases without any change to the DT bindings.
[snip]
> > > diff --git a/drivers/media/i2c/max2175/max2175.c
> > > b/drivers/media/i2c/max2175/max2175.c new file mode 100644 index
> > > 0000000..71b60c2
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/max2175/max2175.c
[snip]
> > > +static int max2175_poll_csm_ready(struct max2175_ctx *ctx) {
> > > + return max2175_poll_timeout(ctx, 69, 1, 1, 0, 50);
> >
> > Please define macros for register addresses and values, this is just
> > unreadable.
>
> The Tuner provider is unwilling to disclose all register details. I agree on
> the readability issue with this restriction but this is somewhat true for
> some sensitive IPs in the media subsystem.
Is it the case that you don't have access to the information, or that you have
been forbidden to disclose them by the tuner manufacturer ?
> > > +}
[snip]
> > > +static int max2175_set_lo_freq(struct max2175_ctx *ctx, u64 lo_freq)
> > > +{
> > > + int ret;
> > > + u32 lo_mult;
> > > + u64 scaled_lo_freq;
> > > + const u64 scale_factor = 1000000ULL;
> > > + u64 scaled_npf, scaled_integer, scaled_fraction;
> > > + u32 frac_desired, int_desired;
> > > + u8 loband_bits, vcodiv_bits;
> >
> > Do you really support frequencies above 4GHz ?
>
> Nope.
>
> If not most of the 64-bit
>
> > values could be stored in 32 bits.
>
> The 64bit variables are needed to extract the fractional part (upto 6 digit
> precision) out of floating point divisions (original user space code).
OK. The code would be more efficient if you made the scaling factor a power of
two though. 1048576 could be a good value.
> >> +
> >> + scaled_lo_freq = lo_freq;
> >> + /* Scale to larger number for precision */
> >> + scaled_lo_freq = scaled_lo_freq * scale_factor * 100;
I just noticed that you could write the two lines as
scaled_lo_freq = lo_freq * scale_factor * 100;
By the way, why do you multiply by 100 here, and...
> >> + mxm_dbg(ctx, "scaled lo_freq %llu lo_freq %llu\n",
> >> + scaled_lo_freq, lo_freq);
> >> +
> >> + if (MAX2175_IS_BAND_AM(ctx)) {
> >> + if (max2175_get_bit(ctx, 5, 7) == 0)
> >> + loband_bits = 0;
> >> + vcodiv_bits = 0;
> >> + lo_mult = 16;
> >> + } else if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_FM) {
> >> + if (lo_freq <= 74700000) {
> >> + loband_bits = 0;
> >> + vcodiv_bits = 0;
> >> + lo_mult = 16;
> >> + } else if ((lo_freq > 74700000) && (lo_freq <= 110000000)) {
> >
> > No need for the inner parentheses.
>
> Agreed.
>
> >> + loband_bits = 1;
> >> + vcodiv_bits = 0;
> >> + } else {
> >> + loband_bits = 1;
> >> + vcodiv_bits = 3;
> >> + }
> >> + lo_mult = 8;
> >> + } else if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_VHF) {
> >> + if (lo_freq <= 210000000) {
> >> + loband_bits = 2;
> >> + vcodiv_bits = 2;
> >> + } else {
> >> + loband_bits = 2;
> >> + vcodiv_bits = 1;
> >> + }
> >> + lo_mult = 4;
> >> + } else {
> >> + loband_bits = 3;
> >> + vcodiv_bits = 2;
> >> + lo_mult = 2;
> >> + }
> >> +
> >> + if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_L)
> >> + scaled_npf = (scaled_lo_freq / ctx->xtal_freq / lo_mult) /
> >> 100;
> >> + else
> >> + scaled_npf = (scaled_lo_freq / ctx->xtal_freq * lo_mult) /
> >> 100;
... divide by 100 here without using the value in-between nor after ?
> >> +
> >> + scaled_integer = scaled_npf / scale_factor * scale_factor;
> >> + int_desired = (u32)(scaled_npf / scale_factor);
> >> + scaled_fraction = scaled_npf - scaled_integer;
> >> + frac_desired = (u32)(scaled_fraction * 1048576 / scale_factor);
> >> +
> >> + /* Check CSM is not busy */
> >> + ret = max2175_poll_csm_ready(ctx);
> >> + if (ret) {
> >> + v4l2_err(ctx->client, "lo_freq: csm busy. freq %llu\n",
> >> + lo_freq);
> >> + return ret;
> >> + }
> >> +
> >> + mxm_dbg(ctx, "loband %u vcodiv %u lo_mult %u scaled_npf %llu\n",
> >> + loband_bits, vcodiv_bits, lo_mult, scaled_npf);
> >> + mxm_dbg(ctx, "scaled int %llu frac %llu desired int %u frac %u\n",
> >> + scaled_integer, scaled_fraction, int_desired, frac_desired);
> >> +
> >> + /* Write the calculated values to the appropriate registers */
> >> + max2175_set_bits(ctx, 5, 3, 2, loband_bits);
> >> + max2175_set_bits(ctx, 6, 7, 6, vcodiv_bits);
> >> + max2175_set_bits(ctx, 1, 7, 0, (u8)(int_desired & 0xff));
> >> + max2175_set_bits(ctx, 2, 3, 0, (u8)((frac_desired >> 16) & 0x1f));
> >> + max2175_set_bits(ctx, 3, 7, 0, (u8)((frac_desired >> 8) & 0xff));
> >> + max2175_set_bits(ctx, 4, 7, 0, (u8)(frac_desired & 0xff));
> >> + /* Flush the above registers to device */
> >> + max2175_flush_regstore(ctx, 1, 6);
> >> + return ret;
> >> +}
[snip]
> >> +static int max2175_get_lna_gain(struct max2175_ctx *ctx) {
> >> + int gain = 0;
> >> + enum max2175_band band = max2175_get_bits(ctx, 5, 1, 0);
> >> +
> >> + switch (band) {
> >> + case MAX2175_BAND_AM:
> >> + gain = max2175_read_bits(ctx, 51, 3, 1);
> >> + break;
> >> + case MAX2175_BAND_FM:
> >> + gain = max2175_read_bits(ctx, 50, 3, 1);
> >> + break;
> >> + case MAX2175_BAND_VHF:
> >> + gain = max2175_read_bits(ctx, 52, 3, 0);
> >> + break;
> >> + default:
> >> + v4l2_err(ctx->client, "invalid band %d to get rf gain\n",
> >> band);
> >
> > Can this happen ?
>
> Yes, there is "L-band". It is a paranoia check as I am testing by comparing
> logs sometimes :-(
OK. By the way, you could get rid of the gain variable by returning directly
in the case statements.
> >> + break;
> >> + }
> >> + return gain;
> >> +}
[snip]
> >> +static const struct v4l2_ctrl_config max2175_i2s_mode = {
> >> + .ops = &max2175_ctrl_ops,
> >> + .id = V4L2_CID_MAX2175_I2S_MODE,
> >> + .name = "I2S_MODE value",
> >> + .type = V4L2_CTRL_TYPE_INTEGER,
> >
> > Should this be a menu control ?
>
> Hmm... the strings would be named "i2s mode x"? Will that be OK?
How about describing the modes instead ? Are they standardized ?
> >> + .min = 0,
> >> + .max = 4,
> >> + .step = 1,
> >> + .def = 0,
> >> +};
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-10 8:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Mark Rutland
Cc: catalin.marinas, will.deacon, robh+dt, bhelgaas, olof, arnd,
linux-arm-kernel, lorenzo.pieralisi, linux-kernel, linuxarm,
devicetree, linux-pci, linux-serial, minyard, liviu.dudau,
zourongrong, john.garry, gabriele.paoloni, zhichang.yuan02,
kantyzc, xuwei5, marc.zyngier
In-Reply-To: <1478647002.7430.69.camel@kernel.crashing.org>
Hi, Ben,
On 2016/11/9 7:16, Benjamin Herrenschmidt wrote:
> On Tue, 2016-11-08 at 12:03 +0000, Mark Rutland wrote:
>> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
>>>
>>> For arm64, there is no I/O space as other architectural platforms, such as
>>> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
>>> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
>>> known port addresses are used to control the corresponding target devices, for
>>> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
>>> normal MMIO mode in using.
>>
>> This has nothing to do with arm64. Hardware with this kind of indirect
>> bus access could be integrated with a variety of CPU architectures. It
>> simply hasn't been, yet.
>
> On some ppc's we also use similar indirect access methods for IOs. We
> have a generic infrastructure for re-routing some memory or IO regions
> to hooks.
>
I am interested on the generic infrastructure on PPC.
Could you point out where those drivers are?
want to take a look..
Thanks,
Zhichang
> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
> firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
>
>>> To drive these devices, this patch introduces a method named indirect-IO.
>>> In this method the in/out pair in arch/arm64/include/asm/io.h will be
>>> redefined. When upper layer drivers call in/out with those known legacy port
>>> addresses to access the peripherals, the hooking functions corrresponding to
>>> those target peripherals will be called. Through this way, those upper layer
>>> drivers which depend on in/out can run on Hip06 without any changes.
>>
>> As above, this has nothing to do with arm64, and as such, should live in
>> generic code, exactly as we would do if we had higher-level ISA
>> accessor ops.
>>
>> Regardless, given the multi-instance case, I don't think this is
>> sufficient in general (and I think we need higher-level ISA accessors
>> to handle the indirection).
>
> Multi-instance with IO is tricky to do generically because archs already
> have all sort of hacks to deal with the fact that inb/outb don't require
> an explicit ioremap, so an IO resource can take all sort of shape depending
> on the arch.
>
> Overall it boils down to applying some kind of per-instance "offset" to
> the IO port number though.
>
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Thomas Petazzoni @ 2016-11-10 8:22 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Andrew Lunn, Jason Cooper, devicetree, Rob Herring,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161110001000.10619-5-gregory.clement@free-electrons.com>
Hello,
On Thu, 10 Nov 2016 01:09:50 +0100, Gregory CLEMENT wrote:
> - bootrom {
> + bootrom@0 {
> compatible = "marvell,bootrom";
> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
I am still not sure whether this "0" unit address is correct compared
to the reg property being passed.
A good example of why I'm worried is the sa-sram case:
+ crypto_sram0: sa-sram0@0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
+ crypto_sram1: sa-sram1@0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
The node names should be just "sram" without a number. Indeed for UARTs
for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
uart1@ABC. But then, if you do that, with your scheme, you end up with
both nodes named sa-sram@0.
Which clearly shows that the way you set this unit-address is not
correct: those two devices are mapped at completely different
locations, but you end up with an identical unit address.
I have no idea what is the rule for setting the unit address in this
case, but I'm pretty sure the rule you've chosen is not good.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 4/5] doc_rst: media: New SDR formats SC16, SC18 & SC20
From: Laurent Pinchart @ 2016-11-10 8:18 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: robh+dt, mark.rutland, mchehab, hverkuil, sakari.ailus, crope,
chris.paterson2, geert+renesas, linux-media, devicetree,
linux-renesas-soc
In-Reply-To: <1478706284-59134-5-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>
Hi Ramesh,
Thank you for the patch.
On Wednesday 09 Nov 2016 15:44:43 Ramesh Shanmugasundaram wrote:
> This patch adds documentation for the three new SDR formats
>
> V4L2_SDR_FMT_SCU16BE
> V4L2_SDR_FMT_SCU18BE
> V4L2_SDR_FMT_SCU20BE
>
> Signed-off-by: Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com>
> ---
> .../media/uapi/v4l/pixfmt-sdr-scu16be.rst | 80 ++++++++++++++++++
> .../media/uapi/v4l/pixfmt-sdr-scu18be.rst | 80 ++++++++++++++++++
> .../media/uapi/v4l/pixfmt-sdr-scu20be.rst | 80 ++++++++++++++++++
> Documentation/media/uapi/v4l/sdr-formats.rst | 3 +
> 4 files changed, 243 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
>
> diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst new file mode 100644
> index 0000000..7525378
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> @@ -0,0 +1,80 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-SDR-FMT-SCU16BE:
> +
> +******************************
> +V4L2_SDR_FMT_SCU16BE ('SC16')
> +******************************
> +
> +Sliced complex unsigned 16-bit big endian IQ sample
> +
> +Description
> +===========
> +
> +This format contains a sequence of complex number samples. Each complex
> +number consist of two parts called In-phase and Quadrature (IQ). Both I
> +and Q are represented as a 16 bit unsigned big endian number stored in
> +32 bit space. The remaining unused bits within the 32 bit space will be
> +padded with 0. I value starts first and Q value starts at an offset
> +equalling half of the buffer size (i.e.) offset = buffersize/2. Out of
> +the 16 bits, bit 15:2 (14 bit) is data and bit 1:0 (2 bit) can be any
> +value.
I've pinged Antti and Hans regarding single buffer vs. multiplanar, let's try
to reach an agreement there.
> +
> +**Byte Order.**
> +Each cell is one byte.
> +
> +.. flat-table::
> + :header-rows: 1
> + :stub-columns: 0
> +
> + * - Offset:
> +
In the meantime, you can remove all the blank lines between table rows :-)
> + - Byte B0
> +
> + - Byte B1
> +
> + - Byte B2
> +
> + - Byte B3
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/2] backlight: arcxcnn: add support for ArticSand devices
From: Lee Jones @ 2016-11-10 8:14 UTC (permalink / raw)
To: Olimpiu Dejeu
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
jingoohan1-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20161109155307.GH13127@dell>
On Wed, 09 Nov 2016, Lee Jones wrote:
> Jingoo?
Ah, it looks like you used the wrong address.
I should have been: jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
I'll CC him from here.
> On Wed, 26 Oct 2016, Olimpiu Dejeu wrote:
>
> > Resubmition of arcxcnn backliught driver adding devicetree entries
> > for all registers
> >
> > Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> >
> > ---
> > drivers/video/backlight/Kconfig | 7 +
> > drivers/video/backlight/Makefile | 1 +
> > drivers/video/backlight/arcxcnn_bl.c | 541 +++++++++++++++++++++++++++++++++++
> > include/linux/i2c/arcxcnn.h | 67 +++++
> > 4 files changed, 616 insertions(+)
> > create mode 100644 drivers/video/backlight/arcxcnn_bl.c
> > create mode 100644 include/linux/i2c/arcxcnn.h
> >
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 5ffa4b4..4e1d2ad 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -460,6 +460,13 @@ config BACKLIGHT_BD6107
> > help
> > If you have a Rohm BD6107 say Y to enable the backlight driver.
> >
> > +config BACKLIGHT_ARCXCNN
> > + tristate "Backlight driver for the Arctic Sands ARCxCnnnn family"
> > + depends on I2C
> > + help
> > + If you have an ARCxCnnnn family backlight say Y to enable
> > + the backlight driver.
> > +
> > endif # BACKLIGHT_CLASS_DEVICE
> >
> > endif # BACKLIGHT_LCD_SUPPORT
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index 16ec534..8905129 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -55,3 +55,4 @@ obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
> > obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
> > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> > +obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
> > diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c
> > new file mode 100644
> > index 0000000..1dad680
> > --- /dev/null
> > +++ b/drivers/video/backlight/arcxcnn_bl.c
> > @@ -0,0 +1,541 @@
> > +/*
> > + * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
> > + *
> > + * Copyright 2016 ArcticSand, Inc.
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/backlight.h>
> > +#include <linux/err.h>
> > +#include <linux/of.h>
> > +#include <linux/pwm.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include "linux/i2c/arcxcnn.h"
> > +
> > +#define ARCXCNN_CMD (0x00) /* Command Register */
> > +#define ARCXCNN_CMD_STDBY (0x80) /* I2C Standby */
> > +#define ARCXCNN_CMD_RESET (0x40) /* Reset */
> > +#define ARCXCNN_CMD_BOOST (0x10) /* Boost */
> > +#define ARCXCNN_CMD_OVP_MASK (0x0C) /* --- Over Voltage Threshold */
> > +#define ARCXCNN_CMD_OVP_XXV (0x0C) /* <rsvrd> Over Voltage Threshold */
> > +#define ARCXCNN_CMD_OVP_20V (0x08) /* 20v Over Voltage Threshold */
> > +#define ARCXCNN_CMD_OVP_24V (0x04) /* 24v Over Voltage Threshold */
> > +#define ARCXCNN_CMD_OVP_31V (0x00) /* 31.4v Over Voltage Threshold */
> > +#define ARCXCNN_CMD_EXT_COMP (0x01) /* part (0) or full (1) external comp */
> > +
> > +#define ARCXCNN_CONFIG (0x01) /* Configuration */
> > +#define ARCXCNN_STATUS1 (0x02) /* Status 1 */
> > +#define ARCXCNN_STATUS2 (0x03) /* Status 2 */
> > +#define ARCXCNN_FADECTRL (0x04) /* Fading Control */
> > +#define ARCXCNN_ILED_CONFIG (0x05) /* ILED Configuration */
> > +
> > +#define ARCXCNN_LEDEN (0x06) /* LED Enable Register */
> > +#define ARCXCNN_LEDEN_ISETEXT (0x80) /* Full-scale current set externally */
> > +#define ARCXCNN_LEDEN_MASK (0x3F) /* LED string enables */
> > +#define ARCXCNN_LEDEN_LED1 (0x01)
> > +#define ARCXCNN_LEDEN_LED2 (0x02)
> > +#define ARCXCNN_LEDEN_LED3 (0x04)
> > +#define ARCXCNN_LEDEN_LED4 (0x08)
> > +#define ARCXCNN_LEDEN_LED5 (0x10)
> > +#define ARCXCNN_LEDEN_LED6 (0x20)
> > +
> > +#define ARCXCNN_WLED_ISET_LSB (0x07) /* LED ISET LSB (in upper nibble) */
> > +#define ARCXCNN_WLED_ISET_MSB (0x08) /* LED ISET MSB (8 bits) */
> > +
> > +#define ARCXCNN_DIMFREQ (0x09)
> > +#define ARCXCNN_COMP_CONFIG (0x0A)
> > +#define ARCXCNN_FILT_CONFIG (0x0B)
> > +#define ARCXCNN_IMAXTUNE (0x0C)
> > +
> > +#define DEFAULT_BL_NAME "arctic_bl"
> > +#define MAX_BRIGHTNESS 4095
> > +
> > +static int s_no_reset_on_remove;
> > +module_param_named(noreset, s_no_reset_on_remove, int, 0644);
> > +MODULE_PARM_DESC(noreset, "No reset on module removal");
> > +
> > +static int s_ibright = 60;
> > +module_param_named(ibright, s_ibright, int, 0644);
> > +MODULE_PARM_DESC(ibright, "Initial brightness (when no plat data)");
> > +
> > +static int s_iledstr = 0x3F;
> > +module_param_named(iledstr, s_iledstr, int, 0644);
> > +MODULE_PARM_DESC(iledstr, "Initial LED String (when no plat data)");
> > +
> > +static int s_retries = 2; /* 1 == only one try */
> > +module_param_named(retries, s_retries, int, 0644);
> > +MODULE_PARM_DESC(retries, "I2C retries attempted");
> > +
> > +enum arcxcnn_brightness_ctrl_mode {
> > + PWM_BASED = 1,
> > + REGISTER_BASED,
> > +};
> > +
> > +struct arcxcnn;
> > +
> > +struct arcxcnn {
> > + char chipname[64];
> > + enum arcxcnn_chip_id chip_id;
> > + enum arcxcnn_brightness_ctrl_mode mode;
> > + struct i2c_client *client;
> > + struct backlight_device *bl;
> > + struct device *dev;
> > + struct arcxcnn_platform_data *pdata;
> > + struct pwm_device *pwm;
> > + struct regulator *supply; /* regulator for VDD input */
> > +};
> > +
> > +static int arcxcnn_write_byte(struct arcxcnn *lp, u8 reg, u8 data)
> > +{
> > + s32 ret = -1;
> > + int att;
> > +
> > + for (att = 0; att < s_retries; att++) {
> > + ret = i2c_smbus_write_byte_data(lp->client, reg, data);
> > + if (ret >= 0)
> > + return 0;
> > + }
> > + return ret;
> > +}
> > +
> > +static u8 arcxcnn_read_byte(struct arcxcnn *lp, u8 reg)
> > +{
> > + int val;
> > + int att;
> > +
> > + for (att = 0; att < s_retries; att++) {
> > + val = i2c_smbus_read_byte_data(lp->client, reg);
> > + if (val >= 0)
> > + return (u8)val;
> > + }
> > + return 0;
> > +}
> > +
> > +static int arcxcnn_update_bit(struct arcxcnn *lp, u8 reg, u8 mask, u8 data)
> > +{
> > + int ret, att;
> > + u8 tmp;
> > +
> > + for (att = 0, ret = -1; att < s_retries; att++) {
> > + ret = i2c_smbus_read_byte_data(lp->client, reg);
> > + if (ret >= 0)
> > + break;
> > + }
> > + if (ret < 0) {
> > + dev_err(lp->dev, "failed to read 0x%.2x\n", reg);
> > + return ret;
> > + }
> > +
> > + tmp = (u8)ret;
> > + tmp &= ~mask;
> > + tmp |= data & mask;
> > +
> > + return arcxcnn_write_byte(lp, reg, tmp);
> > +}
> > +
> > +static int arcxcnn_set_brightness(struct arcxcnn *lp, u32 brightness)
> > +{
> > + int ret;
> > + u8 val;
> > +
> > + val = (brightness & 0xF) << 4;
> > + ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_LSB, val);
> > + if (ret < 0)
> > + return ret;
> > + val = (brightness >> 4);
> > + ret = arcxcnn_write_byte(lp, ARCXCNN_WLED_ISET_MSB, val);
> > + return ret;
> > +}
> > +
> > +static int arcxcnn_bl_update_status(struct backlight_device *bl)
> > +{
> > + struct arcxcnn *lp = bl_get_data(bl);
> > + u32 brightness = bl->props.brightness;
> > +
> > + if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > + brightness = 0;
> > +
> > + /* set brightness */
> > + if (lp->mode == PWM_BASED)
> > + ; /* via pwm */
> > + else if (lp->mode == REGISTER_BASED)
> > + arcxcnn_set_brightness(lp, brightness);
> > +
> > + /* set power-on/off/save modes */
> > + if (bl->props.power == 0)
> > + /* take out of standby */
> > + arcxcnn_update_bit(lp, ARCXCNN_CMD, ARCXCNN_CMD_STDBY, 0);
> > + else
> > + /* 1-3 == power save, 4 = off
> > + * place in low-power standby mode
> > + */
> > + arcxcnn_update_bit(lp, ARCXCNN_CMD,
> > + ARCXCNN_CMD_STDBY, ARCXCNN_CMD_STDBY);
> > + return 0;
> > +}
> > +
> > +static const struct backlight_ops arcxcnn_bl_ops = {
> > + .options = BL_CORE_SUSPENDRESUME,
> > + .update_status = arcxcnn_bl_update_status,
> > +};
> > +
> > +static int arcxcnn_backlight_register(struct arcxcnn *lp)
> > +{
> > + struct backlight_device *bl;
> > + struct backlight_properties props;
> > + struct arcxcnn_platform_data *pdata = lp->pdata;
> > + const char *name = pdata->name ? : DEFAULT_BL_NAME;
> > +
> > + memset(&props, 0, sizeof(props));
> > + props.type = BACKLIGHT_PLATFORM;
> > + props.max_brightness = MAX_BRIGHTNESS;
> > +
> > + if (pdata->initial_brightness > props.max_brightness)
> > + pdata->initial_brightness = props.max_brightness;
> > +
> > + props.brightness = pdata->initial_brightness;
> > +
> > + bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> > + &arcxcnn_bl_ops, &props);
> > + if (IS_ERR(bl))
> > + return PTR_ERR(bl);
> > +
> > + lp->bl = bl;
> > +
> > + return 0;
> > +}
> > +
> > +static ssize_t arcxcnn_get_chip_id(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct arcxcnn *lp = dev_get_drvdata(dev);
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", lp->chipname);
> > +}
> > +
> > +static ssize_t arcxcnn_get_led_str(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct arcxcnn *lp = dev_get_drvdata(dev);
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%02X\n", lp->pdata->led_str);
> > +}
> > +
> > +static ssize_t arcxcnn_set_led_str(struct device *dev,
> > + struct device_attribute *attr, const char *buf, size_t len)
> > +{
> > + struct arcxcnn *lp = dev_get_drvdata(dev);
> > + unsigned long ledstr;
> > +
> > + if (kstrtoul(buf, 0, &ledstr))
> > + return 0;
> > +
> > + if (ledstr != lp->pdata->led_str) {
> > + /* don't allow 0 for ledstr, use power to turn all off */
> > + if (ledstr == 0)
> > + return 0;
> > + lp->pdata->led_str = ledstr & 0x3F;
> > + arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> > + ARCXCNN_LEDEN_MASK, lp->pdata->led_str);
> > + }
> > + return len;
> > +}
> > +
> > +static ssize_t arcxcnn_get_bl_ctl_mode(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct arcxcnn *lp = dev_get_drvdata(dev);
> > + char *strmode = NULL;
> > +
> > + if (lp->mode == PWM_BASED)
> > + strmode = "pwm based";
> > + else if (lp->mode == REGISTER_BASED)
> > + strmode = "register based";
> > +
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", strmode);
> > +}
> > +
> > +static DEVICE_ATTR(chip_id, 0444, arcxcnn_get_chip_id, NULL);
> > +static DEVICE_ATTR(led_str, 0664, arcxcnn_get_led_str, arcxcnn_set_led_str);
> > +static DEVICE_ATTR(bl_ctl_mode, 0444, arcxcnn_get_bl_ctl_mode, NULL);
> > +
> > +static struct attribute *arcxcnn_attributes[] = {
> > + &dev_attr_chip_id.attr,
> > + &dev_attr_led_str.attr,
> > + &dev_attr_bl_ctl_mode.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group arcxcnn_attr_group = {
> > + .attrs = arcxcnn_attributes,
> > +};
> > +
> > +#ifdef CONFIG_OF
> > +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> > +{
> > + struct device *dev = lp->dev;
> > + struct device_node *node = dev->of_node;
> > + u32 prog_val, num_entry, sources[6];
> > + int ret;
> > +
> > + if (!node) {
> > + dev_err(dev, "no platform data.\n");
> > + return -EINVAL;
> > + }
> > + lp->pdata->led_config_0_set = false;
> > + lp->pdata->led_config_1_set = false;
> > + lp->pdata->dim_freq_set = false;
> > + lp->pdata->comp_config_set = false;
> > + lp->pdata->filter_config_set = false;
> > + lp->pdata->trim_config_set = false;
> > +
> > + ret = of_property_read_string(node, "label", &lp->pdata->name);
> > + if (ret < 0)
> > + lp->pdata->name = NULL;
> > +
> > + ret = of_property_read_u32(node, "default-brightness", &prog_val);
> > + if (ret < 0)
> > + prog_val = s_ibright;
> > + lp->pdata->initial_brightness = prog_val;
> > + if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
> > + lp->pdata->initial_brightness = MAX_BRIGHTNESS;
> > +
> > + ret = of_property_read_u32(node, "arcticsand,led-config-0", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->led_config_0 = (u8)prog_val;
> > + lp->pdata->led_config_0_set = true;
> > + }
> > + ret = of_property_read_u32(node, "arcticsand,led-config-1", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->led_config_1 = (u8)prog_val;
> > + lp->pdata->led_config_1_set = true;
> > + }
> > + ret = of_property_read_u32(node, "arcticsand,dim-freq", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->dim_freq = (u8)prog_val;
> > + lp->pdata->dim_freq_set = true;
> > + }
> > + ret = of_property_read_u32(node, "arcticsand,comp-config", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->comp_config = (u8)prog_val;
> > + lp->pdata->comp_config_set = true;
> > + }
> > + ret = of_property_read_u32(node, "arcticsand,filter-config", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->filter_config = (u8)prog_val;
> > + lp->pdata->filter_config_set = true;
> > + }
> > + ret = of_property_read_u32(node, "arcticsand,trim-config", &prog_val);
> > + if (ret == 0) {
> > + lp->pdata->trim_config = (u8)prog_val;
> > + lp->pdata->trim_config_set = true;
> > + }
> > + ret = of_property_count_u32_elems(node, "led-sources");
> > + if (ret < 0)
> > + lp->pdata->led_str = 0x3F;
> > + else {
> > + num_entry = ret;
> > + if (num_entry > 6)
> > + num_entry = 6;
> > +
> > + ret = of_property_read_u32_array(node, "led-sources", sources,
> > + num_entry);
> > + if (ret < 0) {
> > + dev_err(dev, "led-sources node is invalid.\n");
> > + return -EINVAL;
> > + }
> > +
> > + lp->pdata->led_str = 0;
> > + while (num_entry > 0)
> > + lp->pdata->led_str |= (1 << sources[--num_entry]);
> > + }
> > + return 0;
> > +}
> > +#else
> > +static int arcxcnn_parse_dt(struct arcxcnn *lp)
> > +{
> > + return -EINVAL;
> > +}
> > +#endif
> > +
> > +static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> > +{
> > + struct arcxcnn *lp;
> > + int ret;
> > + u8 regval;
> > + u16 chipid;
> > +
> > + if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> > + return -EIO;
> > +
> > + lp = devm_kzalloc(&cl->dev, sizeof(*lp), GFP_KERNEL);
> > + if (!lp)
> > + return -ENOMEM;
> > +
> > + lp->client = cl;
> > + lp->dev = &cl->dev;
> > + lp->chip_id = id->driver_data;
> > + lp->pdata = dev_get_platdata(&cl->dev);
> > +
> > + if (!lp->pdata) {
> > + lp->pdata = devm_kzalloc(lp->dev,
> > + sizeof(*lp->pdata), GFP_KERNEL);
> > + if (!lp->pdata)
> > + return -ENOMEM;
> > +
> > + /* no platform data, parse the device-tree for info. if there
> > + * is no device tree entry, we are being told we exist because
> > + * user-land said so, so make up the info we need
> > + */
> > + ret = arcxcnn_parse_dt(lp);
> > + if (ret < 0) {
> > + /* no device tree, use defaults based on module params
> > + */
> > + lp->pdata->led_config_0_set = false;
> > + lp->pdata->led_config_1_set = false;
> > + lp->pdata->dim_freq_set = false;
> > + lp->pdata->comp_config_set = false;
> > + lp->pdata->filter_config_set = false;
> > + lp->pdata->trim_config_set = false;
> > +
> > + lp->pdata->name = NULL;
> > + lp->pdata->initial_brightness = s_ibright;
> > + lp->pdata->led_str = s_iledstr;
> > + }
> > + }
> > +
> > + if (lp->pdata->dim_freq_set)
> > + lp->mode = PWM_BASED;
> > + else
> > + lp->mode = REGISTER_BASED;
> > +
> > + i2c_set_clientdata(cl, lp);
> > +
> > + /* read device ID */
> > + regval = arcxcnn_read_byte(lp, 0x1E);
> > + chipid = regval;
> > + chipid <<= 8;
> > + regval = arcxcnn_read_byte(lp, 0x1F);
> > + chipid |= regval;
> > +
> > + /* make sure it belongs to this driver
> > + * TODO - handle specific ids
> > + */
> > + if (chipid != 0x02A5) {
> > + #if 1
> > + dev_info(&cl->dev, "Chip Id is %04X\n", chipid);
> > + #else
> > + dev_err(&cl->dev, "%04X is not ARC2C\n", chipid);
> > + return -ENODEV;
> > + #endif
> > + }
> > + /* reset the device */
> > + arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> > +
> > + /* set initial brightness */
> > + arcxcnn_set_brightness(lp, lp->pdata->initial_brightness);
> > +
> > + /* if fadectrl set in DT, set the value directly, else leave default */
> > + if (lp->pdata->led_config_0_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_FADECTRL,
> > + lp->pdata->led_config_0);
> > +
> > + /* if iled config set in DT, set the value, else internal mode */
> > + if (lp->pdata->led_config_1_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG,
> > + lp->pdata->led_config_1);
> > + else
> > + arcxcnn_write_byte(lp, ARCXCNN_ILED_CONFIG, 0x57);
> > +
> > + /* other misc DT settings */
> > + if (lp->pdata->dim_freq_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_FADECTRL, lp->pdata->dim_freq);
> > + if (lp->pdata->comp_config_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_COMP_CONFIG,
> > + lp->pdata->comp_config);
> > + if (lp->pdata->filter_config_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_FILT_CONFIG,
> > + lp->pdata->filter_config);
> > + if (lp->pdata->trim_config_set)
> > + arcxcnn_write_byte(lp, ARCXCNN_IMAXTUNE,
> > + lp->pdata->trim_config);
> > +
> > + /* set initial LED Strings */
> > + arcxcnn_update_bit(lp, ARCXCNN_LEDEN,
> > + ARCXCNN_LEDEN_MASK, lp->pdata->led_str);
> > +
> > + snprintf(lp->chipname, sizeof(lp->chipname),
> > + "%s-%04X", id->name, chipid);
> > +
> > + ret = arcxcnn_backlight_register(lp);
> > + if (ret) {
> > + dev_err(lp->dev,
> > + "failed to register backlight. err: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = sysfs_create_group(&lp->dev->kobj, &arcxcnn_attr_group);
> > + if (ret) {
> > + dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + backlight_update_status(lp->bl);
> > + return 0;
> > +}
> > +
> > +static int arcxcnn_remove(struct i2c_client *cl)
> > +{
> > + struct arcxcnn *lp = i2c_get_clientdata(cl);
> > +
> > + if (!s_no_reset_on_remove) {
> > + /* disable all strings */
> > + arcxcnn_write_byte(lp, ARCXCNN_LEDEN, 0x00);
> > + /* reset the device */
> > + arcxcnn_write_byte(lp, ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> > + }
> > + lp->bl->props.brightness = 0;
> > + backlight_update_status(lp->bl);
> > + if (lp->supply)
> > + regulator_disable(lp->supply);
> > + sysfs_remove_group(&lp->dev->kobj, &arcxcnn_attr_group);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id arcxcnn_dt_ids[] = {
> > + { .compatible = "arc,arc2c0608" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);
> > +
> > +/* Note that the device/chip ID is not fixed in silicon so
> > + * auto-probing of these devices on the bus is most likely
> > + * not possible, use device tree to set i2c bus address
> > + */
> > +static const struct i2c_device_id arcxcnn_ids[] = {
> > + {"arc2c0608", ARC2C0608},
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, arcxcnn_ids);
> > +
> > +static struct i2c_driver arcxcnn_driver = {
> > + .driver = {
> > + .name = "arcxcnn_bl",
> > + .of_match_table = of_match_ptr(arcxcnn_dt_ids),
> > + },
> > + .probe = arcxcnn_probe,
> > + .remove = arcxcnn_remove,
> > + .id_table = arcxcnn_ids,
> > +};
> > +
> > +module_i2c_driver(arcxcnn_driver);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Brian Dodge <bdodge09-1ViLX0X+lBJBDgjK7y7TUQ@public.gmane.org>");
> > +MODULE_DESCRIPTION("ARCXCNN Backlight driver");
> > diff --git a/include/linux/i2c/arcxcnn.h b/include/linux/i2c/arcxcnn.h
> > new file mode 100644
> > index 0000000..1c681dd
> > --- /dev/null
> > +++ b/include/linux/i2c/arcxcnn.h
> > @@ -0,0 +1,67 @@
> > +/*
> > + * Backlight driver for ArcticSand ARC2C0608 Backlight Devices
> > + *
> > + * Copyright 2016 ArcticSand, Inc.
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +
> > +#ifndef _ARCXCNN_H
> > +#define _ARCXCNN_H
> > +
> > +enum arcxcnn_chip_id {
> > + ARC2C0608
> > +};
> > +
> > +enum arcxcnn_brightness_source {
> > + ARCXCNN_PWM_ONLY,
> > + ARCXCNN_I2C_ONLY = 2,
> > +};
> > +
> > +#define ARCXCNN_MAX_PROGENTRIES 48 /* max a/v pairs for custom */
> > +
> > +/**
> > + * struct arcxcnn_platform_data
> > + * @name : Backlight driver name. If it is not defined, default name is set.
> > + * @initial_brightness : initial value of backlight brightness
> > + * @led_str : initial LED string enables, upper bit is global on/off
> > + * @led_config_0 : fading speed (period between intensity steps)
> > + * @led_config_1 : misc settings, see datasheet
> > + * @dim_freq : pwm dimming frequency if in pwm mode
> > + * @comp_config : misc config, see datasheet
> > + * @filter_config: RC/PWM filter config, see datasheet
> > + * @trim_config : full scale current trim, see datasheet
> > + * @led_config_0_set : the value in led_config_0 is valid
> > + * @led_config_1_set : the value in led_config_1 is valid
> > + * @dim_freq_set : the value in dim_freq is valid
> > + * @comp_config_set : the value in comp_config is valid
> > + * @filter_config_set : the value in filter_config is valid
> > + * @trim_config_set : the value in trim_config is valid
> > + *
> > + * the _set flags are used to indicate that the value was explicitly set
> > + * in the device tree or platform data. settings not set are left as default
> > + * power-on default values of the chip except for led_str and led_config_1
> > + * which are set by the driver (led_str is specified indirectly in the
> > + * device tree via "led-sources")
> > + */
> > +struct arcxcnn_platform_data {
> > + const char *name;
> > + u16 initial_brightness;
> > + u8 led_str;
> > +
> > + u8 led_config_0;
> > + u8 led_config_1;
> > + u8 dim_freq;
> > + u8 comp_config;
> > + u8 filter_config;
> > + u8 trim_config;
> > +
> > + bool led_config_0_set;
> > + bool led_config_1_set;
> > + bool dim_freq_set;
> > + bool comp_config_set;
> > + bool filter_config_set;
> > + bool trim_config_set;
> > +};
> > +
> > +#endif
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [RFC 5/5] doc_rst: media: New SDR formats SC16, SC18 & SC20
From: Laurent Pinchart @ 2016-11-10 8:08 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: Antti Palosaari, robh+dt@kernel.org, mark.rutland@arm.com,
mchehab@kernel.org, hverkuil@xs4all.nl,
sakari.ailus@linux.intel.com, Chris Paterson,
geert@linux-m68k.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <SG2PR06MB103893781A3AC3FAB8389DB8C3A20@SG2PR06MB1038.apcprd06.prod.outlook.com>
Antti, Hans, ping ? Please see below.
On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
> > On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
> >> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
> >>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> >>>>>
> >>>>>> This patch adds documentation for the three new SDR formats
> >>>>>>
> >>>>>> V4L2_SDR_FMT_SCU16BE
> >>>>>> V4L2_SDR_FMT_SCU18BE
> >>>>>> V4L2_SDR_FMT_SCU20BE
> >>>
> >>> [snip]
> >>>
> >>>>>> +
> >>>>>> + - start + 0:
> >>>>>> +
> >>>>>> + - I'\ :sub:`0[D13:D6]`
> >>>>>> +
> >>>>>> + - I'\ :sub:`0[D5:D0]`
> >>>>>> +
> >>>>>> + - .. row 2
> >>>>>> +
> >>>>>> + - start + buffer_size/2:
> >>>>>> +
> >>>>>> + - Q'\ :sub:`0[D13:D6]`
> >>>>>> +
> >>>>>> + - Q'\ :sub:`0[D5:D0]`
> >>>>>
> >>>>>
> >>>>>
> >>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
> >>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
> >>>>> formats.
> >>>>
> >>>> Thank you for bringing up this topic. This is one of the key design
> >>>> dilemma.
> >>>>
> >>>> The I & Q data for these three SDR formats comes from two different
> >>>> DMA channels and hence two separate pointers -> we could say it is
> >>>> v4l2 multi- planar. Right now, I am making it look like a single
> >>>> plane by presenting the data in one single buffer ptr.
> >>>>
> >>>> For e.g. multi-planar SC16 format would look something like this
> >>>>
> >>>> <------------------------32bits---------------------->
> >>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
> >>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
> >>>> ...
> >>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
> >>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
> >>>>
> >>>> My concerns are
> >>>>
> >>>> 1) These formats are not a standard as the video "Image Formats".
> >>>> These formats are possible when we use DRIF + MAX2175 combination.
> >>>> If we interface with a different tuner vendor, the above format(s)
> >>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
> >>>> main open item for discussion in the cover letter.
> >>
> >> If the formats are really device-specific then they should be
> >> documented accordingly and not made generic.
> >>
> >>>> 2) MPLANE support within V4L2 seems specific to video. Please
> >>>> correct me if this is wrong interpretation.
> >>>>
> >>>> - struct v4l2_format contains v4l2_sdr_format and
> >>>> v4l2_pix_format_mplane as members of union. Should I create a new
> >>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
> >>>> of the video specific members would be unused (it would be similar
> >>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
> >>
> >> I have no answer to that question as I'm not familiar with SDR. Antti,
> >> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
> >> as you've acked the patch, your input would be appreciated as well.
> >
> > If I understood correctly this hardware provides I and Q samples via
> > different channels and driver now combines those channels as a sequential
> > IQ sample pairs.
>
> The driver combines the two buffer ptrs and present as one single buffer.
> For a buffer of size 200
>
> ptr + 0 : I I I I ... I
> ptr + 100 : Q Q Q Q ... Q
>
> > I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
>
> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
> Those modes are not added in the RFC patchset.
>
> > This is
> > I I I I ... I
> > Q Q Q Q ... Q
> > I am not very familiar with planars, but it sounds like it is correct
> > approach. So I think should be added rather than emulate packet
> > sequential format.
>
> My understanding of V4L2 MPLANE constructs is limited to a quick code read
> only. At this point MPLANE support seems specific to video. SDR is defined
> as separate format like v4l2_pix_format. Questions would be - should we
> define new SDR_MPLANE? or merge SDR format with pix format & reuse existing
> MPLANE with some SDR extensions (if possible)? These seem big design
> decisions. Any suggestions please?
>
> For my use case, MPLANE support does not seem to add significant benefit
> except it may be syntactically correct. I am doing cyclic DMA with a small
> set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer
> at two offsets. If we add MPLANE support, it can be two non-contiguous
> buffer pointers.
>
> >>>> - The above decision (accomodate SDR & MPLANE) needs to be
> >>>> propagated across the framework. Is this the preferred approach?
> >>>>
> >>>> It goes back to point (1). As of today, the change set for this
> >>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
> >>>> further SDR+MPLANE support to the framework as well?
> >>>>
> >>>> I would appreciate your suggestions on this regard.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v4] regulator: lp873x: Add support for populating input supply
From: Lee Jones @ 2016-11-10 8:06 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Linux OMAP Mailing List, broonie-DgEjT+Ai2ygdnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Tero Kristo, Linux ARM Mailing List, Sekhar Nori
In-Reply-To: <20161110052915.4898-1-lokeshvutla-l0cyMroinI0@public.gmane.org>
On Thu, 10 Nov 2016, Lokesh Vutla wrote:
> In order to have a proper topology of regulators for a platform, each
> registering regulator needs to populate supply_name field for identifying
> its supply's name. Add supply_name field for lp873x regulators.
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> ---
> Changes since v3:
> - Applied Rob's ack.
> - Sending this patch separately so that this can be merged via Regulator tree.
> - Link to v3: https://patchwork.kernel.org/patch/9408545/
>
> Documentation/devicetree/bindings/mfd/lp873x.txt | 8 ++++++++
For my own reference:
Acked-for-MFD-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> drivers/regulator/lp873x-regulator.c | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/lp873x.txt b/Documentation/devicetree/bindings/mfd/lp873x.txt
> index 52766c2..ae9cf39 100644
> --- a/Documentation/devicetree/bindings/mfd/lp873x.txt
> +++ b/Documentation/devicetree/bindings/mfd/lp873x.txt
> @@ -7,6 +7,9 @@ Required properties:
> - #gpio-cells: Should be two. The first cell is the pin number and
> the second cell is used to specify flags.
> See ../gpio/gpio.txt for more information.
> + - xxx-in-supply: Phandle to parent supply node of each regulator
> + populated under regulators node. xxx can be
> + buck0, buck1, ldo0 or ldo1.
> - regulators: List of child nodes that specify the regulator
> initialization data.
> Example:
> @@ -17,6 +20,11 @@ pmic: lp8733@60 {
> gpio-controller;
> #gpio-cells = <2>;
>
> + buck0-in-supply = <&vsys_3v3>;
> + buck1-in-supply = <&vsys_3v3>;
> + ldo0-in-supply = <&vsys_3v3>;
> + ldo1-in-supply = <&vsys_3v3>;
> +
> regulators {
> lp8733_buck0: buck0 {
> regulator-name = "lp8733-buck0";
> diff --git a/drivers/regulator/lp873x-regulator.c b/drivers/regulator/lp873x-regulator.c
> index e504b91..70e3df6 100644
> --- a/drivers/regulator/lp873x-regulator.c
> +++ b/drivers/regulator/lp873x-regulator.c
> @@ -24,6 +24,7 @@
> [_id] = { \
> .desc = { \
> .name = _name, \
> + .supply_name = _of "-in", \
> .id = _id, \
> .of_match = of_match_ptr(_of), \
> .regulators_node = of_match_ptr("regulators"),\
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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/3] firmware: qcom: scm: add a video command for state setting
From: Stanimir Varbanov @ 2016-11-10 8:05 UTC (permalink / raw)
To: Stephen Boyd, Stanimir Varbanov
Cc: Ohad Ben-Cohen, Bjorn Andersson, Andy Gross, Rob Herring,
Mark Rutland, Srinivas Kandagatla, linux-remoteproc, linux-kernel,
linux-arm-msm, linux-soc, devicetree
In-Reply-To: <20161110015251.GC16026@codeaurora.org>
Hi,
On 11/10/2016 03:52 AM, Stephen Boyd wrote:
> On 11/07, Stanimir Varbanov wrote:
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index d95c70227c05..7e364691a87c 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
>> }
>> EXPORT_SYMBOL(qcom_scm_is_available);
>>
>> +int qcom_scm_video_set_state(u32 state, u32 spare)
>> +{
>> + int ret;
>> +
>> + ret = qcom_scm_clk_enable();
>
> Do we need clk control for this? Usually it's only required for
> crypto engine things, and turning on video doesn't sound like it
> uses crypto. I don't think downstream android kernel does this.
Correct, the crypto clk is not needed.
--
regards,
Stan
^ permalink raw reply
* [PATCH] ARM: dts: at91: sama5d3_uart: fix reg sizes to match documentation
From: Peter Rosin @ 2016-11-10 7:46 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Nicolas Ferre, Alexandre Belloni,
Jean-Christophe Plagniol-Villard, Rob Herring, Mark Rutland,
Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
The new size (0x100) also matches the size given in sama5d3.dtsi
Documentation reference: section 43.6 "Universal Asynchronous
Receiver Transmitter (UART) User Interface", table 43-4 "Register
Mapping" in [1].
[1] Atmel-11121F-ATARM-SAMA5D3-Series-Datasheet_02-Feb-16
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
arch/arm/boot/dts/sama5d3_uart.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/sama5d3_uart.dtsi b/arch/arm/boot/dts/sama5d3_uart.dtsi
index 2511d748867b..186377d41c91 100644
--- a/arch/arm/boot/dts/sama5d3_uart.dtsi
+++ b/arch/arm/boot/dts/sama5d3_uart.dtsi
@@ -55,7 +55,7 @@
uart0: serial@f0024000 {
compatible = "atmel,at91sam9260-usart";
- reg = <0xf0024000 0x200>;
+ reg = <0xf0024000 0x100>;
interrupts = <16 IRQ_TYPE_LEVEL_HIGH 5>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart0>;
@@ -66,7 +66,7 @@
uart1: serial@f8028000 {
compatible = "atmel,at91sam9260-usart";
- reg = <0xf8028000 0x200>;
+ reg = <0xf8028000 0x100>;
interrupts = <17 IRQ_TYPE_LEVEL_HIGH 5>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1>;
--
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
* Re: [PATCH v3 5/6] Documentation: bindings: add documentation for ir-spi device driver
From: Jacek Anaszewski @ 2016-11-10 7:46 UTC (permalink / raw)
To: Rob Herring
Cc: Andi Shyti, Mauro Carvalho Chehab, Sean Young, Mark Rutland,
Richard Purdie, linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161109182621.ttfxtdt32q3cqce7@rob-hp-laptop>
On 11/09/2016 07:26 PM, Rob Herring wrote:
> On Thu, Nov 03, 2016 at 11:39:21AM +0100, Jacek Anaszewski wrote:
>> On 11/03/2016 11:10 AM, Andi Shyti wrote:
>>> Hi Jacek,
>>>
>>>> Only DT bindings of LED class drivers should be placed in
>>>> Documentation/devicetree/bindings/leds. Please move it to the
>>>> media bindings.
>>>
>>> that's where I placed it first, but Rob asked me to put it in the
>>> LED directory and Cc the LED mailining list.
>>>
>>> That's the discussion of the version 2:
>>>
>>> https://lkml.org/lkml/2016/9/12/380
>>>
>>> Rob, Jacek, could you please agree where I can put the binding?
>>
>> I'm not sure if this is a good approach. I've noticed also that
>> backlight bindings have been moved to leds, whereas they don't look
>> similarly.
>>
>> We have common.txt LED bindings, that all LED class drivers' bindings
>> have to follow. Neither backlight bindings nor these ones do that,
>> which introduces some mess.
>
> And there are probably LED bindings that don't follow common.txt either.
>
>> Eventually adding a sub-directory, e.g. remote_control could make it
>> somehow logically justified, but still - shouldn't bindings be
>> placed in the documentation directory related to the subsystem of the
>> driver they are predestined to?
>
> No. While binding directories often mirror the driver directories, they
> are not the same. Bindings are grouped by types of h/w and IR LEDs are a
> type of LED.
>
> If you prefer a sub-dir, that is fine with me.
Fine. So how about sub-dir "ir" ?
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] mfd: pm8921: add support to pm8821
From: kbuild test robot @ 2016-11-10 7:41 UTC (permalink / raw)
Cc: kbuild-all, Lee Jones, Rob Herring, Andy Gross, devicetree,
linux-kernel, linux-arm-msm, linux-soc, linux-arm-kernel,
bjorn.andersson, Srinivas Kandagatla
In-Reply-To: <1478622577-20699-1-git-send-email-srinivas.kandagatla@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]
Hi Srinivas,
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.9-rc4 next-20161110]
[cannot apply to robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/mfd-pm8921-add-support-to-pm8821/20161109-013248
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arm-pxa_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
drivers/mfd/pm8921-core.c: In function 'pm8921_probe':
>> drivers/mfd/pm8921-core.c:630:58: warning: dereferencing 'void *' pointer
data = of_match_node(pm8921_id_table, pdev->dev.of_node)->data;
^~
>> drivers/mfd/pm8921-core.c:630:58: error: request for member 'data' in something not a structure or union
vim +/data +630 drivers/mfd/pm8921-core.c
624 const struct pm8xxx_data *data;
625 int irq, rc;
626 unsigned int val;
627 u32 rev;
628 struct pm_irq_chip *chip;
629
> 630 data = of_match_node(pm8921_id_table, pdev->dev.of_node)->data;
631 if (!data) {
632 dev_err(&pdev->dev, "No matching driver data found\n");
633 return -EINVAL;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30088 bytes --]
^ permalink raw reply
* Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Benjamin Herrenschmidt @ 2016-11-10 7:08 UTC (permalink / raw)
To: Mark Rutland
Cc: zhichang.yuan, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, olof-nZhT3qVonbNeoWH0uzbU5w,
arnd-r2nGTMty4D4,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
lorenzo.pieralisi-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, minyard-HInyCGIudOg,
liviu.dudau-5wv7dgnIgG8, zourongrong-Re5JQEeQqe8AvxtiuMwx3w,
john.garry-hv44wF8Li93QT0dZR+AlfA,
gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
zhichang.yuan02-Re5JQEeQqe8AvxtiuMwx3w, kantyzc-9Onoh4P/yGk,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, marc.zyngier-5wv7dgnIgG8
In-Reply-To: <20161109111959.GB17020@leverpostej>
On Wed, 2016-11-09 at 11:20 +0000, Mark Rutland wrote:
> The big change would be to handle !MMIO translations, for which we'd
> need a runtime registry of ISA bus instance to find the relevant
> accessor ops and instance-specific data.
Yes. We do something a bit like that on ppc, we find the PCI bus
for which the IO ports match and I have a hook to register the
special indirect ISA.
It's a bit messy, we could do something nicer generically.
Cheers,
Ben.
--
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 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-10 6:40 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Gabriele Paoloni, mark.rutland-5wv7dgnIgG8@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org,
minyard-HInyCGIudOg@public.gmane.org,
catalin.marinas-5wv7dgnIgG8@public.gmane.org, John Garry,
will.deacon-5wv7dgnIgG8@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, xuwei (O),
Linuxarm, olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
zourongrong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-fy+rA21nqHI
In-Reply-To: <2825537.ADCNsGqGxn@wuerfel>
Hi, Arnd,
On 2016/11/10 5:34, Arnd Bergmann wrote:
> On Wednesday, November 9, 2016 12:10:43 PM CET Gabriele Paoloni wrote:
>>> On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote:
>>>> + /*
>>>> + * The first PCIBIOS_MIN_IO is reserved specifically for
>>> indirectIO.
>>>> + * It will separate indirectIO range from pci host bridge to
>>>> + * avoid the possible PIO conflict.
>>>> + * Set the indirectIO range directly here.
>>>> + */
>>>> + lpcdev->io_ops.start = 0;
>>>> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1;
>>>> + lpcdev->io_ops.devpara = lpcdev;
>>>> + lpcdev->io_ops.pfin = hisilpc_comm_in;
>>>> + lpcdev->io_ops.pfout = hisilpc_comm_out;
>>>> + lpcdev->io_ops.pfins = hisilpc_comm_ins;
>>>> + lpcdev->io_ops.pfouts = hisilpc_comm_outs;
>>>
>>> I have to look at patch 2 in more detail again, after missing a few
>>> review
>>> rounds. I'm still a bit skeptical about hardcoding a logical I/O port
>>> range here, and would hope that we can just go through the same
>>> assignment of logical port ranges that we have for PCI buses,
>>> decoupling
>>> the bus addresses from the linux-internal ones.
>>
>> The point here is that we want to avoid any conflict/overlap between
>> the LPC I/O space and the PCI I/O space. With the assignment above
>> we make sure that LPC never interfere with PCI I/O space.
>
> But we already abstract the PCI I/O space using dynamic registration.
> There is no need to hardcode the logical address for ISA, though
> I think we can hardcode the bus address to start at zero here.
Do you means that we can pick up the maximal I/O address from all children's
device resources??
Thanks,
Zhichang
>
> Arnd
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: zhichang.yuan @ 2016-11-10 6:24 UTC (permalink / raw)
To: liviu.dudau@arm.com, Gabriele Paoloni
Cc: catalin.marinas@arm.com, will.deacon@arm.com, robh+dt@kernel.org,
bhelgaas@google.com, mark.rutland@arm.com, olof@lixom.net,
arnd@arndb.de, linux-arm-kernel@lists.infradead.org,
lorenzo.pieralisi@arm.com, linux-kernel@vger.kernel.org, Linuxarm,
devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
linux-serial@vger.kernel.org, minyard@acm.org, benh
In-Reply-To: <20161109165044.GE10219@e106497-lin.cambridge.arm.com>
Hi,Liviu,
Thanks for your comments!
On 2016/11/10 0:50, liviu.dudau@arm.com wrote:
> On Wed, Nov 09, 2016 at 04:16:17PM +0000, Gabriele Paoloni wrote:
>> Hi Liviu
>>
>> Thanks for reviewing
>>
>
> [removed some irrelevant part of discussion, avoid crazy formatting]
>
>>>> +/**
>>>> + * addr_is_indirect_io - check whether the input taddr is for
>>> indirectIO.
>>>> + * @taddr: the io address to be checked.
>>>> + *
>>>> + * Returns 1 when taddr is in the range; otherwise return 0.
>>>> + */
>>>> +int addr_is_indirect_io(u64 taddr)
>>>> +{
>>>> + if (arm64_extio_ops->start > taddr || arm64_extio_ops->end <
>>> taddr)
>>>
>>> start >= taddr ?
>>
>> Nope... if (taddr < arm64_extio_ops->start || taddr > arm64_extio_ops->end)
>> then taddr is outside the range [start; end] and will return 0; otherwise
>> it will return 1...
>
> Oops, sorry, did not pay attention to the returned value. The check is
> correct as it is, no need to change then.
>
>>
>>>
>>>> + return 0;
>>>> +
>>>> + return 1;
>>>> +}
>>>>
>>>> BUILD_EXTIO(b, u8)
>>>>
>>>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>>>> index 02b2903..cc2a05d 100644
>>>> --- a/drivers/of/address.c
>>>> +++ b/drivers/of/address.c
>>>> @@ -479,6 +479,50 @@ static int of_empty_ranges_quirk(struct
>>> device_node *np)
>>>> return false;
>>>> }
>>>>
>>>> +
>>>> +/*
>>>> + * of_isa_indirect_io - get the IO address from some isa reg
>>> property value.
>>>> + * For some isa/lpc devices, no ranges property in ancestor node.
>>>> + * The device addresses are described directly in their regs
>>> property.
>>>> + * This fixup function will be called to get the IO address of
>>> isa/lpc
>>>> + * devices when the normal of_translation failed.
>>>> + *
>>>> + * @parent: points to the parent dts node;
>>>> + * @bus: points to the of_bus which can be used to parse
>>> address;
>>>> + * @addr: the address from reg property;
>>>> + * @na: the address cell counter of @addr;
>>>> + * @presult: store the address paresed from @addr;
>>>> + *
>>>> + * return 1 when successfully get the I/O address;
>>>> + * 0 will return for some failures.
>>>
>>> Bah, you are returning a signed int, why 0 for failure? Return a
>>> negative value with
>>> error codes. Otherwise change the return value into a bool.
>>
>> Yes we'll move to bool
>>
>>>
>>>> + */
>>>> +static int of_get_isa_indirect_io(struct device_node *parent,
>>>> + struct of_bus *bus, __be32 *addr,
>>>> + int na, u64 *presult)
>>>> +{
>>>> + unsigned int flags;
>>>> + unsigned int rlen;
>>>> +
>>>> + /* whether support indirectIO */
>>>> + if (!indirect_io_enabled())
>>>> + return 0;
>>>> +
>>>> + if (!of_bus_isa_match(parent))
>>>> + return 0;
>>>> +
>>>> + flags = bus->get_flags(addr);
>>>> + if (!(flags & IORESOURCE_IO))
>>>> + return 0;
>>>> +
>>>> + /* there is ranges property, apply the normal translation
>>> directly. */
>>>
>>> s/there is ranges/if we have a 'ranges'/
>>
>> Thanks for spotting this
>>
>>>
>>>> + if (of_get_property(parent, "ranges", &rlen))
>>>> + return 0;
>>>> +
>>>> + *presult = of_read_number(addr + 1, na - 1);
>>>> + /* this fixup is only valid for specific I/O range. */
>>>> + return addr_is_indirect_io(*presult);
>>>> +}
>>>> +
>>>> static int of_translate_one(struct device_node *parent, struct
>>> of_bus *bus,
>>>> struct of_bus *pbus, __be32 *addr,
>>>> int na, int ns, int pna, const char *rprop)
>>>> @@ -595,6 +639,15 @@ static u64 __of_translate_address(struct
>>> device_node *dev,
>>>> result = of_read_number(addr, na);
>>>> break;
>>>> }
>>>> + /*
>>>> + * For indirectIO device which has no ranges property, get
>>>> + * the address from reg directly.
>>>> + */
>>>> + if (of_get_isa_indirect_io(dev, bus, addr, na, &result)) {
>>>> + pr_debug("isa indirectIO matched(%s)..addr =
>>> 0x%llx\n",
>>>> + of_node_full_name(dev), result);
>>>> + break;
>>>> + }
>>>>
>>>> /* Get new parent bus and counts */
>>>> pbus = of_match_bus(parent);
>>>> @@ -688,8 +741,9 @@ static int __of_address_to_resource(struct
>>> device_node *dev,
>>>> if (taddr == OF_BAD_ADDR)
>>>> return -EINVAL;
>>>> memset(r, 0, sizeof(struct resource));
>>>> - if (flags & IORESOURCE_IO) {
>>>> + if (flags & IORESOURCE_IO && taddr >= PCIBIOS_MIN_IO) {
>>>> unsigned long port;
>>>> +
>>>> port = pci_address_to_pio(taddr);
>>>> if (port == (unsigned long)-1)
>>>> return -EINVAL;
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index ba34907..1a08511 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -3263,7 +3263,7 @@ int __weak pci_register_io_range(phys_addr_t
>>> addr, resource_size_t size)
>>>>
>>>> #ifdef PCI_IOBASE
>>>> struct io_range *range;
>>>> - resource_size_t allocated_size = 0;
>>>> + resource_size_t allocated_size = PCIBIOS_MIN_IO;
>>>>
>>>> /* check if the range hasn't been previously recorded */
>>>> spin_lock(&io_range_lock);
>>>> @@ -3312,7 +3312,7 @@ phys_addr_t pci_pio_to_address(unsigned long
>>> pio)
>>>>
>>>> #ifdef PCI_IOBASE
>>>> struct io_range *range;
>>>> - resource_size_t allocated_size = 0;
>>>> + resource_size_t allocated_size = PCIBIOS_MIN_IO;
>>>
>>> Have you checked that pci_pio_to_address still returns valid values
>>> after this? I know that
>>> you are trying to take into account PCIBIOS_MIN_IO limit when
>>> allocating reserving the IO ranges,
>>> but the values added in the io_range_list are still starting from zero,
>>> no from PCIBIOS_MIN_IO,
>>
>> I think you're wrong here as in pci_address_to_pio we have:
>> + resource_size_t offset = PCIBIOS_MIN_IO;
>>
>> This should be enough to guarantee that the PIOs start at
>> PCIBIOS_MIN_IO...right?
>
> I don't think you can guarantee that the pio value that gets passed into
> pci_pio_to_address() always comes from a previously returned value by
> pci_address_to_pio(). Maybe you can add a check in pci_pio_to_address()
>
> if (pio < PCIBIOS_MIN_IO)
> return address;
>
> to avoid adding more checks in the list_for_each_entry() loop.
>
I will register some ranges to the list and test it later.
But from my understanding, pci_pio_to_address() should can return the right
original physical address.
According to the algorithm, the output PIO ranges are consecutive， just like this:
input pio of pci_pio_to_address()
|
V
|----------------|--------------------------|------|-----------|
^
|
allocated_size is here
The change of this patch just make the start PIO from ZERO to PCIBIOS_MIN_IO.
in pci_pio_to_address(), for one input pio which fall into any PIO segment, the
return address will be:
address = range->start + pio - allocated_size;
Since allocated_size is the total range size of all IO ranges before the one
where pio belong, then (pio - allocated_size) is the offset to the range start,
So....
Thanks!
Zhichang
> Best regards,
> Liviu
>
>>
>>
>>> so the calculation of the address in this function could return
>>> negative values casted to pci_addr_t.
>>>
>>> Maybe you want to adjust the range->start value in
>>> pci_register_io_range() as well to have it
>>> offset by PCIBIOS_MIN_IO as well.
>>>
>>> Best regards,
>>> Liviu
>>>
>>>>
>>>> if (pio > IO_SPACE_LIMIT)
>>>> return address;
>>>> @@ -3335,7 +3335,7 @@ unsigned long __weak
>>> pci_address_to_pio(phys_addr_t address)
>>>> {
>>>> #ifdef PCI_IOBASE
>>>> struct io_range *res;
>>>> - resource_size_t offset = 0;
>>>> + resource_size_t offset = PCIBIOS_MIN_IO;
>>>> unsigned long addr = -1;
>>>>
>>>> spin_lock(&io_range_lock);
>>>> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
>>>> index 3786473..deec469 100644
>>>> --- a/include/linux/of_address.h
>>>> +++ b/include/linux/of_address.h
>>>> @@ -24,6 +24,23 @@ struct of_pci_range {
>>>> #define for_each_of_pci_range(parser, range) \
>>>> for (; of_pci_range_parser_one(parser, range);)
>>>>
>>>> +
>>>> +#ifndef indirect_io_enabled
>>>> +#define indirect_io_enabled indirect_io_enabled
>>>> +static inline bool indirect_io_enabled(void)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +#endif
>>>> +
>>>> +#ifndef addr_is_indirect_io
>>>> +#define addr_is_indirect_io addr_is_indirect_io
>>>> +static inline int addr_is_indirect_io(u64 taddr)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>> /* Translate a DMA address from device space to CPU space */
>>>> extern u64 of_translate_dma_address(struct device_node *dev,
>>>> const __be32 *in_addr);
>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>> index 0e49f70..7f6bbb6 100644
>>>> --- a/include/linux/pci.h
>>>> +++ b/include/linux/pci.h
>>>> @@ -2130,4 +2130,12 @@ static inline bool pci_ari_enabled(struct
>>> pci_bus *bus)
>>>> /* provide the legacy pci_dma_* API */
>>>> #include <linux/pci-dma-compat.h>
>>>>
>>>> +/*
>>>> + * define this macro here to refrain from compilation error for some
>>>> + * platforms. Please keep this macro at the end of this header file.
>>>> + */
>>>> +#ifndef PCIBIOS_MIN_IO
>>>> +#define PCIBIOS_MIN_IO 0
>>>> +#endif
>>>> +
>>>> #endif /* LINUX_PCI_H */
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-pci"
>>> in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCHv2 2/2] gpio: xilinx: Add clock support
From: Shubhrajyoti Datta @ 2016-11-10 6:08 UTC (permalink / raw)
To: Sören Brinkmann
Cc: Shubhrajyoti Datta, linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Michal Simek
In-Reply-To: <20161109171852.GC14444@xsjsorenbubuntu>
On Wed, Nov 9, 2016 at 10:48 PM, Sören Brinkmann
<soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
[...]
>>
>> My idea was to keep the clk name optional since there is only one clock.
>> Or do you think we should mandate the name if clk is provided.
>
> I'd make 'clock-names' mandatory if 'clocks' is present. That way there
> won't be any trouble if this IP ever consumed additional clocks in the
> future.
fixed that in v3.
Thanks
>
> Sören
--
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
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