Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-11-21 20:49 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <58334A06.103-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 11/21/16 11:24, Frank Rowand wrote:
> On 11/21/16 08:23, Sudeep Holla wrote:
>>
>>
>> On 21/11/16 16:05, Frank Rowand wrote:
>>> Hi Sudeep,
>>>
>>> On 11/18/16 12:22, Frank Rowand wrote:
>>>> On 11/18/16 02:41, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> On 17/11/16 21:00, Frank Rowand wrote:
>>>>>> On 11/17/16 07:32, Sudeep Holla wrote:
>>>>>>> Currently platforms/drivers needing to get the machine model name are
>>>>>>> replicating the same snippet of code. In some case, the OF reference
>>>>>>> counting is either missing or incorrect.
>>>>>>>
>>>>>>> This patch adds support to read the machine model name either using
>>>>>>> the "model" or the "compatible" property in the device tree root node
>>>>>>> to the core OF/DT code.
>>>>>>>
>>>>>>> This can be used to remove all the duplicate code snippets doing exactly
>>>>>>> same thing later.
>>>>>>
>>>>>> I find five instances of reading only property "model":
>>>>>>
>>>>>>   arch/arm/mach-imx/cpu.c
>>>>>>   arch/arm/mach-mxs/mach-mxs.c
>>>>>>   arch/c6x/kernel/setup.c
>>>>>>   arch/mips/cavium-octeon/setup.c
>>>>>>   arch/sh/boards/of-generic.c
>>>>>>
>>>>>
>>>>> Ah sorry you were not Cc-ed in 2/2, but that shows all the instances
>>>>> that this will be used for.
>>>>
>>>> I have not seen 2/2.  I do not see it on the devicetree list or on lkml.
>>>
>>> Can you please re-send patch 2/2?
>>>
>>
>> Since it is based on -next, I would prefer to wait until next merge
>> window to resend. You should be able to check in the link I sent if
>> that's OK.
> 
> I am missing or misunderstanding something.
> 
> I do not know what "the link I sent" means.

Ah, the links were in the email you sent before this one, but I read this
one first.  Got it now.


> 
> For some reason, the devicetree mail list and lmkl mail failed to send
> me a copy of patch 2/2.  Or my mail server failed to receive them.  That
> is why I asked you to resend the patch. I just now looked in the devicetree
> archive and found it there.
> 
> So I now can see how you plan to use the new function.
> 
> -Frank
> 
> 
> 

--
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 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Jon Hunter @ 2016-11-21 20:37 UTC (permalink / raw)
  To: Laxman Dewangan, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5832ED69.3090903-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>


On 21/11/16 12:49, Laxman Dewangan wrote:
> 
> On Monday 21 November 2016 04:38 PM, Jon Hunter wrote:
>>>
>>> I had a discussion with the ASIC on this and as per them
>>>      1.8 V nominal is (1.62V, 1.98V)
>>>      3.3 V nominal is (2.97V,3.63V)
>>>
>>> I am working with them to update the TRM document but we can assume that
>>> this information will be there in TRM.
>> My feeling is that if all use-cases today are using either 1.8V or 3.3V,
>> then may be we should not worry about this and only support either 1.8V
>> or 3.3V. I would be more in favour of supporting other voltages if there
>> is a real need.
> 
> Sometimes, the regulator will not return exact 1.8V or 3.3V due to the
> PMIC rail resolution. On such cases, it returns nearest voltage to 1.8V
> or 3.3V.
> That's why the PMIC resolution is considered through IO pad voltage
> tolerances.

Ok, gotcha. I can see that would be the case for when you call
regulator_get_voltage() (ie. in the probe), but what about the notifier?
I imagine that in the notifier, at least for the pre-change case, that
the voltage is the target and not the actual. So I am wondering if we
need to check for a range in the notifier?

>>>>> +    const struct pinctrl_pin_desc *pins_desc;
>>>>> +    int num_pins_desc;
>>>>> +};
>>>>> +
>>>>> +struct tegra_io_pads_regulator_info {
>>>>> +    struct device *dev;
>>>>> +    const struct tegra_io_pads_cfg_info *pads_cfg;
>>>>> +    struct regulator *regulator;
>>>>> +    struct notifier_block regulator_nb;
>>>>> +};
>>>> Is this struct necessary? Seems to be a lot of duplicated information
>>>> from the other structs. Why not add the regulator and regulator_nb to
>>>> the main struct? OK, not all io_pads have a regulator but you are only
>>>> saving one pointer.
>>> Yes, some of IO pads support multi-voltage.
>> Yes, but I am saying why not put this information in the main struct and
>> not bother having yet another struct where half of the information is
>> duplicated.
> 
> For regulator notifier callback, we will need the driver data. If I keep
> this in the main structure then I will not able to get proper structure
> until I make that as global.
> 
> The notifier registration is
>                 ret = devm_regulator_register_notifier(regulator,
> &rinfo->regulator_nb);
> 
> 
> and from the pointer of rinfo->regulator_nb, I will get the rinfo as
> 
>         rinfo = container_of(nb, struct tegra_io_pads_regulator_info,
>                                      regulator_nb);
> 
> if I use this in main structure then I will not be able to get the
> driver data.

Ah yes, you have an array of regulators. Make sense. However, shame to
duplicate some of this data and also would be good to avoid allocating
so much memory in probe for these structs if only a few rails actually
have regulators ...

  tiopi->rinfo = devm_kzalloc(dev, sizeof(*tiopi->rinfo) *
				soc_data->num_pads_cfg, GFP_KERNEL);

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-11-21 20:21 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <0f150e21-ba46-d4f2-98e8-a226eb82ca3e-5wv7dgnIgG8@public.gmane.org>

On 11/21/16 08:20, Sudeep Holla wrote:
> 
> 
> On 18/11/16 20:22, Frank Rowand wrote:
>> On 11/18/16 02:41, Sudeep Holla wrote:
>>>
>>>
>>> On 17/11/16 21:00, Frank Rowand wrote:
>>>> On 11/17/16 07:32, Sudeep Holla wrote:
>>>>> Currently platforms/drivers needing to get the machine model name are
>>>>> replicating the same snippet of code. In some case, the OF reference
>>>>> counting is either missing or incorrect.
>>>>>
>>>>> This patch adds support to read the machine model name either using
>>>>> the "model" or the "compatible" property in the device tree root node
>>>>> to the core OF/DT code.
>>>>>
>>>>> This can be used to remove all the duplicate code snippets doing exactly
>>>>> same thing later.
>>>>
>>>> I find five instances of reading only property "model":
>>>>
>>>>   arch/arm/mach-imx/cpu.c
>>>>   arch/arm/mach-mxs/mach-mxs.c
>>>>   arch/c6x/kernel/setup.c
>>>>   arch/mips/cavium-octeon/setup.c
>>>>   arch/sh/boards/of-generic.c
>>>>
>>>
>>> Ah sorry you were not Cc-ed in 2/2, but that shows all the instances
>>> that this will be used for.
>>
>> I have not seen 2/2.  I do not see it on the devicetree list or on lkml.
>>
> 
> Yes on both [1][2]
> 
>> I did see a list of drivers in the RFC patch that you sent several hours
>> before this patch.
>>
>> In that patch you replaced reading the model name from the _flat_ device
>> tree with the new function in at least one location.  That is not
>> correct.
>>
>>
>>>
>>>> I find one instance of reading property "model", then if
>>>> that does not exist, property "compatible":
>>>>
>>>>   arch/mips/generic/proc.c

Just for completeness, now that I have seen patch 2/2, there is a
second location that currently uses "compatible" if "model" does
not exist: drivers/soc/fsl/guts.c

>>>>
>>>
>>> Correct as you can check in patch 2/2
>>>
>>>> The proposed patch matches the code used in one place, and thus
>>>> current usage does not match the patch description.
>>>>
>>>
>>> Yes, but does it matter ? compatibles are somewhat informative about the
>>> model IMO.
>>
>> Yes it does matter.  That is just sloppy and makes devicetree yet harder
>> to understand.  It hurts clarity.  The new function name says get "model",
>> not get "model" or "first element of the compatible list".

An example of a function name that would not hurt clarity would be
of_model_or_1st_compatible().


>>
> 
> This is a implementation in the Linux and it doesn't change anything in
> DT semantics. I am not able to get your concern.

The existing code in five locations that patch 2/2 changes only attempt
to read the value of property "model".  Changing those five locations
to use of_machine_get_model_name() results in those locations using the
first string of the "compatible" property if "model" does not exist.

The value found is potentially used to determine whether to execute
model specific code.  An example of this is: octeon_pcie_pcibios_map_irq().
Can you guarantee that there is no device tree that does not contain
a "model" property in the root node, but does contains a "compatible"
property in the root node whose first value is "EBH5600"?

I have pasted the relevant code from octeon_pcie_pcibios_map_irq()
below for convenient reference:

int __init octeon_pcie_pcibios_map_irq(const struct pci_dev *dev,
                                       u8 slot, u8 pin)
{
        /*
         * The EBH5600 board with the PCI to PCIe bridge mistakenly
         * wires the first slot for both device id 2 and interrupt
         * A. According to the PCI spec, device id 2 should be C. The
         * following kludge attempts to fix this.
         */
        if (strstr(octeon_board_type_string(), "EBH5600") &&
            dev->bus && dev->bus->parent) {

My point is that it is not possible to review patch 2/2 to verify whether
any change in kernel behavior results from the change, because we do not
have access to all device tree sources.  patch 2/2 is intended to clean
up code, not to change behavior.


> 
>> And using the _first_ element only of the compatible list to determine
>> model is not a good paradigm.  It is yet another hidden, special case,
>> undocumented trap to lure in the unwary.
>>
> 
> The function is documented and again this doesn't enforce anything in
> the bindings. It's just the way it's used by the Linux kernel.
> 
> [...]
> 
>>
>> You also ignored Arnd's comment in reply to your RFC patch.
>>
> 
> OK, all I can see is that Arnd wanted to reuse of_root, which I did.
> Did I miss anything else ?
> 

My mistake, sorry.  I misread the patch.

-Frank
--
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 2/4] usb: dwc2: Add binding for AHB burst
From: John Youn @ 2016-11-21 20:16 UTC (permalink / raw)
  To: Christian Lamparter, Rob Herring
  Cc: Stefan Wahren, John Youn, Felipe Balbi,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland
In-Reply-To: <6396482.jQaH1ArAfZ@debian64>

On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
>>> Hi John,
>>>
>>> Am 17.11.2016 um 00:47 schrieb John Youn:
>>>> Add the "snps,ahb-burst" binding and read it in.
>>>>
>>>> This property controls which burst type to perform on the AHB bus as a
>>>> master in internal DMA mode. This overrides the legacy param value,
>>>> which we need to keep around for now since several platforms use it.
>>>>
>>>> Some platforms may see better or worse performance based on this
>>>> value. The HAPS platform is one example where all INCRx have worse
>>>> performance than INCR.
>>>>
>>>> Other platforms (such as the Canyonlands board) report that the default
>>>> value causes system hangs.
>>>>
>>>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>>>>  drivers/usb/dwc2/core.h                        |  9 +++++
>>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>>>>  3 files changed, 67 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 6c7c2bce..9e7b4b4 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>
>>> according to Documentation/devicetree/bindings/submitting-patches.txt
>>> this change should be a separate patch.
>>>
>>>> @@ -26,6 +26,8 @@ Optional properties:
>>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>>    Refer to usb/generic.txt
>>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
>>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
>>>
>>> This doesn't apply in case of the bcm2835. I would prefer this option is
>>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
>>> this platform").
>>
>> Also, perhaps you should allow that the compatible string can define the 
>> default.
>>
> I hoped you would say that :).
> 
> I've attached a patch (on top of John Youn changes) that does
> just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
> value into the .data, if that's a problem, I can certainly 
> respin the patch and put it in a dedicated struct.
> 
> Regards
> 
> Christian
> ---
> From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Fri, 18 Nov 2016 21:03:19 +0100
> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> 
> This patch adds a of_device_id table which can be used by
> existing devices to supply a ahb-burst value for the platform
> without having to add a "snps,ahb-burst" entry to the dts.
> 
> Note: Adding new devices to this table is discouraged.
>       please consider adding the "snps,ahb-burst" property
>       with the correct configuration to your device tree
>       file instead.
> 
> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index e0fc9aa..51be266 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
>  };
>  
> +/*
> + * This table provides AHB burst configuration for existing
> + * device tree bindings that work poorly with the default setting.
> + *
> + * Note: Adding new devices to this table is discouraged.
> + *	 please consider adding the "snps,ahb-burst" property
> + *	 with the correct configuration to your device tree
> + *	 file instead.
> + */
> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> +	{
> +		.compatible = "amcc,dwc-otg",
> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> +	},
> +};
> +
>  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>  {
>  	struct device_node *node = hsotg->dev->of_node;
> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>  	ret = device_property_read_string(hsotg->dev,
>  					  "snps,ahb-burst", &str);
>  	if (ret < 0) {
> +		const struct of_device_id *match;
> +
> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> +		if (match)
> +			ret = (int)match->data;
> +
>  		return ret;
>  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
>  		dev_warn(hsotg->dev,
> 

Hi Christian,

I'd prefer if you use the binding which requires no extra code in
dwc2.

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 3/3] ARM64: dts: meson-gxbb: add the USB reset also to the second USB PHY
From: Kevin Hilman @ 2016-11-21 20:15 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
	carlo-KA+7E9HrN00dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <CAFBinCBVaQ7SLwQ0M2eJj0iyQuErF1Ev5dtGzKr7itiesy11MQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> writes:

> Hi Kevin,
>
> On Wed, Nov 16, 2016 at 10:35 PM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>> Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> writes:
>>
>>> When the USB PHY driver was introduced the reset framework did not
>>> have support for triggering a reset pulse for shared resets. On GXBB
>>> however there is only one reset line for both PHYs (meaning we have a
>>> shared reset line). With the latest changes to the reset framework and
>>> the corresponding updates to the phy-meson8b-usb2 driver we can now pass
>>> the reset to the second PHY as well.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> Applied.
> Unfortunately I think I put crucial information only in the
> cover-letter's description:
> "the dts patch has a runtime-dependency on patch 1 and 2"

I saw that, but also see that both of those have been queued, so should
land in v4.10 also.

> So please feel free to keep or drop the patch as it is. In case you
> decide drop it I will re-send it for 4.11 (after all the 4.10 stuff is
> done).

IMO, it's fine to keep it.  That means there may be some versions of
linux-next that have the problem where the reset will get asserted
twice, but since that is affecting very few people (probably only you),
I think it's OK, since it will be fine once v4.10-rc1 is released.

If you don't want that, let me know and I'll drop it for now.

Kevin
--
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 0/2] arm64: dts: NS2: Add sdio1, PCI phys
From: Florian Fainelli @ 2016-11-21 19:27 UTC (permalink / raw)
  To: Jon Mason, Rob Herring, Mark Rutland, Florian Fainelli
  Cc: devicetree, bcm-kernel-feedback-list, linux-arm-kernel,
	linux-kernel
In-Reply-To: <1479425103-2119-1-git-send-email-jon.mason@broadcom.com>

On 11/17/2016 03:25 PM, Jon Mason wrote:
> The second SDIO seems to have been forgotten to be enabled in the
> Northstar2 SVK dts.  Also, the PCI PHY entries are missing from the PCI
> bus device tree entries.

Series applied, thanks Jon
-- 
Florian

^ permalink raw reply

* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-11-21 19:24 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55e27115-421e-b67b-eb7a-9b1c3d662711-5wv7dgnIgG8@public.gmane.org>

On 11/21/16 08:23, Sudeep Holla wrote:
> 
> 
> On 21/11/16 16:05, Frank Rowand wrote:
>> Hi Sudeep,
>>
>> On 11/18/16 12:22, Frank Rowand wrote:
>>> On 11/18/16 02:41, Sudeep Holla wrote:
>>>>
>>>>
>>>> On 17/11/16 21:00, Frank Rowand wrote:
>>>>> On 11/17/16 07:32, Sudeep Holla wrote:
>>>>>> Currently platforms/drivers needing to get the machine model name are
>>>>>> replicating the same snippet of code. In some case, the OF reference
>>>>>> counting is either missing or incorrect.
>>>>>>
>>>>>> This patch adds support to read the machine model name either using
>>>>>> the "model" or the "compatible" property in the device tree root node
>>>>>> to the core OF/DT code.
>>>>>>
>>>>>> This can be used to remove all the duplicate code snippets doing exactly
>>>>>> same thing later.
>>>>>
>>>>> I find five instances of reading only property "model":
>>>>>
>>>>>   arch/arm/mach-imx/cpu.c
>>>>>   arch/arm/mach-mxs/mach-mxs.c
>>>>>   arch/c6x/kernel/setup.c
>>>>>   arch/mips/cavium-octeon/setup.c
>>>>>   arch/sh/boards/of-generic.c
>>>>>
>>>>
>>>> Ah sorry you were not Cc-ed in 2/2, but that shows all the instances
>>>> that this will be used for.
>>>
>>> I have not seen 2/2.  I do not see it on the devicetree list or on lkml.
>>
>> Can you please re-send patch 2/2?
>>
> 
> Since it is based on -next, I would prefer to wait until next merge
> window to resend. You should be able to check in the link I sent if
> that's OK.

I am missing or misunderstanding something.

I do not know what "the link I sent" means.

For some reason, the devicetree mail list and lmkl mail failed to send
me a copy of patch 2/2.  Or my mail server failed to receive them.  That
is why I asked you to resend the patch. I just now looked in the devicetree
archive and found it there.

So I now can see how you plan to use the new function.

-Frank



--
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

* Applied "spi: sh-msiof: Add support for R-Car M3-W" to the spi tree
From: Mark Brown @ 2016-11-21 19:20 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Mark Brown
In-Reply-To: <1479749095-13548-1-git-send-email-geert+renesas@glider.be>

The patch

   spi: sh-msiof: Add support for R-Car M3-W

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From eb51cffa743de5c78cfbf44f576b0f1eccc784f4 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Mon, 21 Nov 2016 18:24:55 +0100
Subject: [PATCH] spi: sh-msiof: Add support for R-Car M3-W

MSIOF in R-Car M3-W (r8a7796) is handled fine by the existing driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt | 1 +
 drivers/spi/spi-sh-msiof.c                         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index aa005c1d10d9..da6614c63796 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -10,6 +10,7 @@ Required properties:
 			 "renesas,msiof-r8a7792" (R-Car V2H)
 			 "renesas,msiof-r8a7793" (R-Car M2-N)
 			 "renesas,msiof-r8a7794" (R-Car E2)
+			 "renesas,msiof-r8a7796" (R-Car M3-W)
 			 "renesas,msiof-sh73a0" (SH-Mobile AG5)
 - reg                  : A list of offsets and lengths of the register sets for
 			 the device.
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 1de3a772eb7d..0012ad02e569 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -980,6 +980,7 @@ static const struct of_device_id sh_msiof_match[] = {
 	{ .compatible = "renesas,msiof-r8a7792",   .data = &r8a779x_data },
 	{ .compatible = "renesas,msiof-r8a7793",   .data = &r8a779x_data },
 	{ .compatible = "renesas,msiof-r8a7794",   .data = &r8a779x_data },
+	{ .compatible = "renesas,msiof-r8a7796",   .data = &r8a779x_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, sh_msiof_match);
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Ondřej Jirman @ 2016-11-21 18:42 UTC (permalink / raw)
  To: moinejf-Re5JQEeQqe8AvxtiuMwx3w
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20161121191416.706b80c25d476fa002a001b1-GANU6spQydw@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 2102 bytes --]

Dne 21.11.2016 v 19:14 Jean-Francois Moine napsal(a):
> On Mon, 21 Nov 2016 01:54:53 +0100
> Ondřej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org> wrote:
> 
>> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
>>> This patchset series adds HDMI video support to the Allwinner
>>> sun8i SoCs which include the display engine 2 (DE2).
>>> The driver contains the code for the A83T and H3, but it could be
>>> used/extended for other SoCs as the A64, H2 and H5.
>>
>> Hi,
>>
>> I'm trying to test your patches on Orange Pi PC, and I've run into a few
>> issues: (I'm using sunxi-ng with the same patches as last time, to make
>> it work with your driver)
>>
>> 1] I just get pink output on the monitor - there's some signal, but it's
>> pink (or more like magenta).
>>
>> dmesg ouput indicates no error:
>>
>> [    1.887823] [drm] Initialized
>> [    1.888503] sun8i-de2 1000000.de-controller: bound
>> 1c0c000.lcd-controller (ops 0xc0a63894)
>> [    2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
>> 0xc0a63b54)
>> [    2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [    2.057307] [drm] No driver support for vblank timestamp query.
>> [    2.690862] Console: switching to colour frame buffer device 240x67
>> [    2.723059] sun8i-de2 1000000.de-controller: fb0:  frame buffer device
> 	[snip]
> 
> My H3 boards work correctly, except the Orange PI 2 when it cannot read
> the EDID (but it is OK after reboot).
> 
> Did you check if the EDID was correctly read?

EDID is correctly read (I verified that it is the same as with the v5
version of the driver), but there's one difference I noted. v5 says dpms
is Off, while v6 says dpms is On.

> Which resolution do you expect?
> 

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Jean-Francois Moine @ 2016-11-21 18:14 UTC (permalink / raw)
  To: Ondřej Jirman
  Cc: moinejf-Re5JQEeQqe8AvxtiuMwx3w, Dave Airlie, Maxime Ripard,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <1c9a02ff-ce6c-7ad7-36fa-8a2ea0b7675e-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>

On Mon, 21 Nov 2016 01:54:53 +0100
Ondřej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org> wrote:

> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
> > This patchset series adds HDMI video support to the Allwinner
> > sun8i SoCs which include the display engine 2 (DE2).
> > The driver contains the code for the A83T and H3, but it could be
> > used/extended for other SoCs as the A64, H2 and H5.
> 
> Hi,
> 
> I'm trying to test your patches on Orange Pi PC, and I've run into a few
> issues: (I'm using sunxi-ng with the same patches as last time, to make
> it work with your driver)
> 
> 1] I just get pink output on the monitor - there's some signal, but it's
> pink (or more like magenta).
> 
> dmesg ouput indicates no error:
> 
> [    1.887823] [drm] Initialized
> [    1.888503] sun8i-de2 1000000.de-controller: bound
> 1c0c000.lcd-controller (ops 0xc0a63894)
> [    2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
> 0xc0a63b54)
> [    2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [    2.057307] [drm] No driver support for vblank timestamp query.
> [    2.690862] Console: switching to colour frame buffer device 240x67
> [    2.723059] sun8i-de2 1000000.de-controller: fb0:  frame buffer device
	[snip]

My H3 boards work correctly, except the Orange PI 2 when it cannot read
the EDID (but it is OK after reboot).

Did you check if the EDID was correctly read?
Which resolution do you expect?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH 2/2] Add basic support for DW CSI-2 Host IPK
From: Ramiro Oliveira @ 2016-11-21 18:13 UTC (permalink / raw)
  To: Sakari Ailus, Ramiro Oliveira, g
  Cc: robh+dt, mark.rutland, mchehab, devicetree, linux-kernel,
	linux-media, davem, gregkh, geert+renesas, akpm, linux, hverkuil,
	laurent.pinchart+renesas, arnd, sudipm.mukherjee, tiffany.lin,
	minghsiu.tsai, jean-christophe.trotin, andrew-ct.chen,
	simon.horman, songjun.wu, bparrot, CARLOS.PALMINHA
In-Reply-To: <20161117163426.GA13965@valkosipuli.retiisi.org.uk>

Hi Sakari,

Thank you for your feedback.

On 11/17/2016 4:34 PM, Sakari Ailus wrote:
> Hi Ramiro,
> 
> Thank you for the patchset. Please see my comments below.
> 
> If you've got a CSI-2 receiver, why do you use the DV timing presets? What
> do you use those for?
> 

Our CSI-2 Host has two different output types, one that doesn't need any extra
info about the video timings, and one that needs some info about the input video
timings. Since the video timings of any input (Camera) should be somewhat
similar to the DV timing presets I decided to use them.

I'm not sure if this answers your question.

> On Mon, Nov 14, 2016 at 02:20:23PM +0000, Ramiro Oliveira wrote:
>> Add support for basic configuration of the DW CSI-2 Host IPK
>>
>> Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
>> ---
>>  MAINTAINERS                                 |   7 +
>>  drivers/media/platform/Kconfig              |   1 +
>>  drivers/media/platform/Makefile             |   2 +
>>  drivers/media/platform/dwc/Kconfig          |  36 ++
>>  drivers/media/platform/dwc/Makefile         |   3 +
>>  drivers/media/platform/dwc/dw_mipi_csi.c    | 687 +++++++++++++++++++++++
>>  drivers/media/platform/dwc/dw_mipi_csi.h    | 179 ++++++
>>  drivers/media/platform/dwc/plat_ipk.c       | 835 ++++++++++++++++++++++++++++
>>  drivers/media/platform/dwc/plat_ipk.h       |  97 ++++
>>  drivers/media/platform/dwc/plat_ipk_video.h |  97 ++++
>>  drivers/media/platform/dwc/video_device.c   | 741 ++++++++++++++++++++++++
>>  drivers/media/platform/dwc/video_device.h   | 101 ++++
>>  12 files changed, 2786 insertions(+)
>>  create mode 100644 drivers/media/platform/dwc/Kconfig
>>  create mode 100644 drivers/media/platform/dwc/Makefile
>>  create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.c
>>  create mode 100644 drivers/media/platform/dwc/dw_mipi_csi.h
>>  create mode 100644 drivers/media/platform/dwc/plat_ipk.c
>>  create mode 100644 drivers/media/platform/dwc/plat_ipk.h
>>  create mode 100644 drivers/media/platform/dwc/plat_ipk_video.h
>>  create mode 100644 drivers/media/platform/dwc/video_device.c
>>  create mode 100644 drivers/media/platform/dwc/video_device.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6a71422..f54dfd8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10618,6 +10618,13 @@ S:	Maintained
>>  F:	drivers/staging/media/st-cec/
>>  F:	Documentation/devicetree/bindings/media/stih-cec.txt
>>  
>> +SYNOPSYS DESIGNWARE CSI-2 HOST IPK DRIVER
>> +M:	Ramiro Oliveira <roliveir@synopsys.com>
>> +L:	linux-media@vger.kernel.org
>> +T:	git git://linuxtv.org/media_tree.git
>> +S:	Maintained
>> +F:	drivers/media/platform/dwc/
>> +
>>  SYNOPSYS DESIGNWARE DMAC DRIVER
>>  M:	Viresh Kumar <vireshk@kernel.org>
>>  M:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index 754edbf1..6fef760f 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -120,6 +120,7 @@ source "drivers/media/platform/am437x/Kconfig"
>>  source "drivers/media/platform/xilinx/Kconfig"
>>  source "drivers/media/platform/rcar-vin/Kconfig"
>>  source "drivers/media/platform/atmel/Kconfig"
>> +source "drivers/media/platform/dwc/Kconfig"
>>  
>>  config VIDEO_TI_CAL
>>  	tristate "TI CAL (Camera Adaptation Layer) driver"
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index f842933..623f5d2 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -61,6 +61,8 @@ obj-$(CONFIG_VIDEO_RCAR_VIN)		+= rcar-vin/
>>  
>>  obj-$(CONFIG_VIDEO_ATMEL_ISC)		+= atmel/
>>  
>> +obj-$(CONFIG_VIDEO_DWC)			+= dwc/
>> +
>>  ccflags-y += -I$(srctree)/drivers/media/i2c
>>  
>>  obj-$(CONFIG_VIDEO_MEDIATEK_VPU)	+= mtk-vpu/
>> diff --git a/drivers/media/platform/dwc/Kconfig b/drivers/media/platform/dwc/Kconfig
>> new file mode 100644
>> index 0000000..fb8533b
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/Kconfig
>> @@ -0,0 +1,36 @@
>> +config VIDEO_DWC
>> +	bool "Designware Cores CSI-2 IPK (EXPERIMENTAL)"
>> +	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA
>> +	help
>> +	  Say Y here to enable support for the DesignWare Cores CSI-2 Host IP
>> +	  Prototyping Kit.
>> +
>> +if VIDEO_DWC
>> +config DWC_PLATFORM
>> +	tristate "SNPS DWC MIPI CSI2 Host"
>> +	depends on VIDEO_DWC
>> +	help
>> +	  This is the V4L2 plaftorm driver driver for the DWC CSI-2 HOST IPK
>> +
>> +	  To compile this driver as a module, choose M here
>> +
>> +
>> +config DWC_MIPI_CSI2_HOST
>> +	tristate "SNPS DWC MIPI CSI2 Host"
>> +	select GENERIC_PHY
>> +	depends on VIDEO_DWC
>> +	help
>> +	  This is a V4L2 driver for SNPS DWC MIPI-CSI2
>> +
>> +	  To compile this driver as a module, choose M here
>> +
>> +config DWC_VIDEO_DEVICE
>> +	tristate "DWC VIDEO DEVICE"
>> +	select VIDEOBUF2_VMALLOC
>> +	select VIDEOBUF2_DMA_CONTIG
>> +	depends on VIDEO_DWC
>> +	help
>> +	  This is a V4L2 driver for SNPS Video device
>> +	  To compile this driver as a module, choose M here
>> +
>> +endif # VIDEO_DWC
>> diff --git a/drivers/media/platform/dwc/Makefile b/drivers/media/platform/dwc/Makefile
>> new file mode 100644
>> index 0000000..75c74b7
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/Makefile
>> @@ -0,0 +1,3 @@
>> +obj-$(CONFIG_DWC_PLATFORM)		+= plat_ipk.o
>> +obj-$(CONFIG_DWC_MIPI_CSI2_HOST)	+= dw_mipi_csi.o
>> +obj-$(CONFIG_DWC_VIDEO_DEVICE)		+= video_device.o
>> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.c b/drivers/media/platform/dwc/dw_mipi_csi.c
>> new file mode 100644
>> index 0000000..1337a5e
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/dw_mipi_csi.c
>> @@ -0,0 +1,687 @@
>> +/*
>> + * DWC MIPI CSI-2 Host device driver
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +
>> +#include "dw_mipi_csi.h"
>> +
>> +/** @short Driver args: debug */
>> +static unsigned int debug;
>> +module_param(debug, uint, 0644);
>> +MODULE_PARM_DESC(debug, "Activates debug info");
> 
> There should be no need for module specific debug parameters. Please use
> dynamic debug instead.
> 

I'll remove it.

>> +
>> +/*
>> + * Basic IO read and write operations
>> + */
>> +
>> +/**
>> + * @short Video formats supported by the MIPI CSI-2
>> + */
>> +static const struct mipi_fmt dw_mipi_csi_formats[] = {
>> +	{
>> +	 .name = "RAW BGGR 8",
> 
> Do you use the name field for something? If not, I'd suggest to drop it.
> 

No, I guess I don't really use it for anything. I'll remove it.

>> +	 .code = MEDIA_BUS_FMT_SBGGR8_1X8,
>> +	 .depth = 8,
>> +	 },
>> +	{
>> +	 .name = "RAW10",
>> +	 .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
>> +	 .depth = 10,
>> +	 },
>> +	{
>> +	 .name = "RGB565",
>> +	 .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>> +	 .depth = 16,
>> +	 },
>> +	{
>> +	 .name = "BGR565",
>> +	 .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
>> +	 .depth = 16,
>> +	 },
>> +	{
>> +	 .name = "RGB888",
>> +	 .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>> +	 .depth = 24,
>> +	 },
>> +	{
>> +	 .name = "BGR888",
>> +	 .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
>> +	 .depth = 24,
>> +	 },
>> +};
>> +
>> +static struct mipi_csi_dev *
>> +sd_to_mipi_csi_dev(struct v4l2_subdev *sdev)
>> +{
>> +	return container_of(sdev, struct mipi_csi_dev, sd);
>> +}
>> +
>> +void
>> +dw_mipi_csi_write(struct mipi_csi_dev *dev,
>> +		  unsigned int address, unsigned int data)
>> +{
>> +	iowrite32(data, dev->base_address + address);
>> +}
>> +
>> +u32
>> +dw_mipi_csi_read(struct mipi_csi_dev *dev, unsigned long address)
>> +{
>> +	return ioread32(dev->base_address + address);
>> +}
>> +
>> +void
>> +dw_mipi_csi_write_part(struct mipi_csi_dev *dev,
>> +		       unsigned long address, unsigned long data,
>> +		       unsigned char shift, unsigned char width)
>> +{
>> +	u32 mask = (1 << width) - 1;
>> +	u32 temp = dw_mipi_csi_read(dev, address);
>> +
>> +	temp &= ~(mask << shift);
>> +	temp |= (data & mask) << shift;
>> +	dw_mipi_csi_write(dev, address, temp);
>> +}
>> +
>> +static const struct mipi_fmt *
>> +find_dw_mipi_csi_format(struct v4l2_mbus_framefmt *mf)
>> +{
>> +	int i;
> 
> unsigned int
> 

I'll change it.

>> +
>> +	for (i = 0; i < ARRAY_SIZE(dw_mipi_csi_formats); i++)
>> +		if (mf->code == dw_mipi_csi_formats[i].code)
>> +			return &dw_mipi_csi_formats[i];
>> +	return NULL;
>> +}
>> +
>> +int
>> +enable_video_output(struct mipi_csi_dev *dev)
>> +{
>> +	return 0;
>> +}
> 
> This one is unused.
> 
>> +
>> +int
>> +disable_video_output(struct mipi_csi_dev *dev)
>> +{
>> +	phy_power_off(dev->phy);
>> +	return 0;
>> +}
> 
> This one as well.
> 

I'll remove both.

>> +
>> +static void
>> +dw_mipi_csi_reset(struct mipi_csi_dev *dev)
>> +{
>> +	dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
>> +	/* mdelay(1); */
>> +	dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 1);
>> +}
>> +
>> +static int
>> +dw_mipi_csi_mask_irq_power_off(struct mipi_csi_dev *dev)
>> +{
>> +	/* set only one lane (lane 0) as active (ON) */
>> +	dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
>> +
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0);
>> +
>> +	dw_mipi_csi_write(dev, R_CSI2_CTRL_RESETN, 0);
>> +
>> +	return 0;
>> +
>> +}
>> +
>> +static int
>> +dw_mipi_csi_hw_stdby(struct mipi_csi_dev *dev)
>> +{
>> +	/* set only one lane (lane 0) as active (ON) */
>> +	dw_mipi_csi_reset(dev);
>> +
>> +	dw_mipi_csi_write(dev, R_CSI2_N_LANES, 0);
>> +
>> +	phy_init(dev->phy);
>> +
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY_FATAL, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT_FATAL, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_FRAME_FATAL, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PHY, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_PKT, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_LINE, 0xFFFFFFFF);
>> +	dw_mipi_csi_write(dev, R_CSI2_MASK_INT_IPI, 0xFFFFFFFF);
>> +
>> +	return 0;
>> +
>> +}
>> +
>> +void
> 
> Shouldn't this be static? The same applies to many other symbols as well.
> 

You're right, I'll change them all.

>> +dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *dev)
>> +{
>> +	switch (dev->fmt->code) {
>> +	case MEDIA_BUS_FMT_RGB565_2X8_BE:
>> +	case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
>> +		v4l2_dbg(1, debug, &dev->sd, "DT: RGB 565");
>> +		break;
>> +
>> +	case MEDIA_BUS_FMT_RGB888_2X12_LE:
>> +	case MEDIA_BUS_FMT_RGB888_2X12_BE:
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB888);
>> +		v4l2_dbg(1, debug, &dev->sd, "DT: RGB 888");
>> +		break;
>> +	case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW10);
>> +		v4l2_dbg(1, debug, &dev->sd, "DT: RAW 10");
>> +		break;
>> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RAW8);
>> +		v4l2_dbg(1, debug, &dev->sd, "DT: RAW 8");
>> +		break;
>> +	default:
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_DATA_TYPE, CSI_2_RGB565);
>> +		v4l2_dbg(1, debug, &dev->sd, "Error");
>> +		break;
>> +	}
>> +}
>> +
>> +void
>> +__dw_mipi_csi_fill_timings(struct mipi_csi_dev *dev,
>> +			   const struct v4l2_bt_timings *bt)
>> +{
>> +
>> +	if (bt == NULL)
>> +		return;
>> +
>> +	dev->hw.hsa = bt->hsync;
>> +	dev->hw.hbp = bt->hbackporch;
>> +	dev->hw.hsd = bt->hsync;
>> +	dev->hw.htotal = bt->height + bt->vfrontporch +
>> +	    bt->vsync + bt->vbackporch;
>> +	dev->hw.vsa = bt->vsync;
>> +	dev->hw.vbp = bt->vbackporch;
>> +	dev->hw.vfp = bt->vfrontporch;
>> +	dev->hw.vactive = bt->height;
>> +}
> 
> There seem to be quite a few unused functions here.
> 

I've removed everything that wasn't being used.

>> +
>> +void
>> +dw_mipi_csi_start(struct mipi_csi_dev *dev)
>> +{
>> +	const struct v4l2_bt_timings *bt = &v4l2_dv_timings_presets[0].bt;
>> +
>> +	__dw_mipi_csi_fill_timings(dev, bt);
>> +
>> +	dw_mipi_csi_write(dev, R_CSI2_N_LANES, (dev->hw.num_lanes - 1));
>> +	v4l2_dbg(1, debug, &dev->sd, "N Lanes: %d\n", dev->hw.num_lanes);
>> +
>> +	/*IPI Related Configuration */
>> +	if ((dev->hw.output_type == IPI_OUT)
>> +	    || (dev->hw.output_type == BOTH_OUT)) {
>> +
>> +		dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE, dev->hw.ipi_mode,
>> +				       0, 1);
>> +		v4l2_dbg(1, debug, &dev->sd, "IPI MODE: %d\n",
>> +			 dev->hw.ipi_mode);
>> +
>> +		dw_mipi_csi_write_part(dev, R_CSI2_IPI_MODE,
>> +				       dev->hw.ipi_color_mode, 8, 1);
>> +		v4l2_dbg(1, debug, &dev->sd, "Color Mode: %d\n",
>> +			 dev->hw.ipi_color_mode);
>> +
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_VCID, dev->hw.virtual_ch);
>> +		v4l2_dbg(1, debug, &dev->sd, "Virtual Channel: %d\n",
>> +			 dev->hw.virtual_ch);
>> +
>> +		dw_mipi_csi_write_part(dev, R_CSI2_IPI_MEM_FLUSH,
>> +				       dev->hw.ipi_auto_flush, 8, 1);
>> +		v4l2_dbg(1, debug, &dev->sd, "Auto-flush: %d\n",
>> +			 dev->hw.ipi_auto_flush);
>> +
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_HSA_TIME, dev->hw.hsa);
>> +		v4l2_dbg(1, debug, &dev->sd, "HSA: %d\n", dev->hw.hsa);
>> +
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_HBP_TIME, dev->hw.hbp);
>> +		v4l2_dbg(1, debug, &dev->sd, "HBP: %d\n", dev->hw.hbp);
>> +
>> +		dw_mipi_csi_write(dev, R_CSI2_IPI_HSD_TIME, dev->hw.hsd);
>> +		v4l2_dbg(1, debug, &dev->sd, "HSD: %d\n", dev->hw.hsd);
>> +
>> +		if (dev->hw.ipi_mode == AUTO_TIMING) {
>> +			dw_mipi_csi_write(dev, R_CSI2_IPI_HLINE_TIME,
>> +					  dev->hw.htotal);
>> +			v4l2_dbg(1, debug, &dev->sd, "H total: %d\n",
>> +				 dev->hw.htotal);
>> +
>> +			dw_mipi_csi_write(dev, R_CSI2_IPI_VSA_LINES,
>> +					  dev->hw.vsa);
>> +			v4l2_dbg(1, debug, &dev->sd, "VSA: %d\n", dev->hw.vsa);
>> +
>> +			dw_mipi_csi_write(dev, R_CSI2_IPI_VBP_LINES,
>> +					  dev->hw.vbp);
>> +			v4l2_dbg(1, debug, &dev->sd, "VBP: %d\n", dev->hw.vbp);
>> +
>> +			dw_mipi_csi_write(dev, R_CSI2_IPI_VFP_LINES,
>> +					  dev->hw.vfp);
>> +			v4l2_dbg(1, debug, &dev->sd, "VFP: %d\n", dev->hw.vfp);
>> +
>> +			dw_mipi_csi_write(dev, R_CSI2_IPI_VACTIVE_LINES,
>> +					  dev->hw.vactive);
>> +			v4l2_dbg(1, debug, &dev->sd, "V Active: %d\n",
>> +				 dev->hw.vactive);
>> +		}
>> +	}
>> +
>> +	phy_power_on(dev->phy);
>> +}
>> +
>> +static int
>> +dw_mipi_csi_enum_mbus_code(struct v4l2_subdev *sd,
>> +			   struct v4l2_subdev_pad_config *cfg,
>> +			   struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> +	if (code->index >= ARRAY_SIZE(dw_mipi_csi_formats))
>> +		return -EINVAL;
>> +
>> +	code->code = dw_mipi_csi_formats[code->index].code;
>> +	return 0;
>> +}
>> +
>> +static struct mipi_fmt const *
>> +dw_mipi_csi_try_format(struct v4l2_mbus_framefmt *mf)
>> +{
>> +	struct mipi_fmt const *fmt;
>> +
>> +	fmt = find_dw_mipi_csi_format(mf);
>> +	if (fmt == NULL)
>> +		fmt = &dw_mipi_csi_formats[0];
>> +
>> +	mf->code = fmt->code;
>> +	return fmt;
>> +}
>> +
>> +static struct v4l2_mbus_framefmt *
>> +__dw_mipi_csi_get_format(struct mipi_csi_dev *dev,
>> +			 struct v4l2_subdev_pad_config *cfg,
>> +			 enum v4l2_subdev_format_whence which)
>> +{
>> +	if (which == V4L2_SUBDEV_FORMAT_TRY)
>> +		return cfg ? v4l2_subdev_get_try_format(&dev->sd, cfg,
>> +							0) : NULL;
>> +
>> +	return &dev->format;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
>> +		    struct v4l2_subdev_format *fmt)
>> +{
>> +	struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> +	struct mipi_fmt const *dev_fmt;
>> +	struct v4l2_mbus_framefmt *mf;
>> +	int i = 0;
> 
> unsigned int
> 

Changing it.

>> +	const struct v4l2_bt_timings *bt_r = &v4l2_dv_timings_presets[0].bt;
>> +
>> +	mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
>> +
>> +	dev_fmt = dw_mipi_csi_try_format(&fmt->format);
>> +	if (dev_fmt) {
>> +		*mf = fmt->format;
>> +		if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> +			dev->fmt = dev_fmt;
>> +		dw_mipi_csi_set_ipi_fmt(dev);
>> +	}
>> +	while (v4l2_dv_timings_presets[i].bt.width) {
>> +		const struct v4l2_bt_timings *bt =
>> +		    &v4l2_dv_timings_presets[i].bt;
>> +		if (mf->width == bt->width && mf->height == bt->width) {
>> +			__dw_mipi_csi_fill_timings(dev, bt);
>> +			return 0;
>> +		}
>> +		i++;
>> +	}
>> +
>> +	__dw_mipi_csi_fill_timings(dev, bt_r);
>> +	return 0;
>> +
>> +}
>> +
>> +static int
>> +dw_mipi_csi_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_pad_config *cfg,
>> +		    struct v4l2_subdev_format *fmt)
>> +{
>> +	struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> +	struct v4l2_mbus_framefmt *mf;
>> +
>> +	mf = __dw_mipi_csi_get_format(dev, cfg, fmt->which);
>> +	if (!mf)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&dev->lock);
>> +	fmt->format = *mf;
>> +	mutex_unlock(&dev->lock);
>> +	return 0;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> +	struct mipi_csi_dev *dev = sd_to_mipi_csi_dev(sd);
>> +
>> +	if (on) {
>> +		dw_mipi_csi_hw_stdby(dev);
>> +		dw_mipi_csi_start(dev);
>> +	} else {
>> +		dw_mipi_csi_mask_irq_power_off(dev);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> +	struct v4l2_mbus_framefmt *format =
>> +	    v4l2_subdev_get_try_format(sd, fh->pad, 0);
>> +
>> +	format->colorspace = V4L2_COLORSPACE_SRGB;
>> +	format->code = dw_mipi_csi_formats[0].code;
>> +	format->width = MIN_WIDTH;
>> +	format->height = MIN_HEIGHT;
>> +	format->field = V4L2_FIELD_NONE;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops dw_mipi_csi_sd_internal_ops = {
>> +	.open = dw_mipi_csi_open,
>> +};
>> +
>> +static struct v4l2_subdev_core_ops dw_mipi_csi_core_ops = {
>> +	.s_power = dw_mipi_csi_s_power,
>> +};
>> +
>> +static struct v4l2_subdev_pad_ops dw_mipi_csi_pad_ops = {
>> +	.enum_mbus_code = dw_mipi_csi_enum_mbus_code,
>> +	.get_fmt = dw_mipi_csi_get_fmt,
>> +	.set_fmt = dw_mipi_csi_set_fmt,
>> +};
>> +
>> +static struct v4l2_subdev_ops dw_mipi_csi_subdev_ops = {
>> +	.core = &dw_mipi_csi_core_ops,
>> +	.pad = &dw_mipi_csi_pad_ops,
>> +};
>> +
>> +static irqreturn_t
>> +dw_mipi_csi_irq1(int irq, void *dev_id)
>> +{
>> +
>> +	struct mipi_csi_dev *dev = dev_id;
>> +	u32 int_status, ind_status;
>> +	unsigned long flags;
>> +
>> +	int_status = dw_mipi_csi_read(dev, R_CSI2_INTERRUPT);
>> +	spin_lock_irqsave(&dev->slock, flags);
>> +
>> +	if (int_status & CSI2_INT_PHY_FATAL) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY_FATAL);
>> +		pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
> 
> I don't think you should use pr_info() for interrupts. This is basically
> debug information, isn't it?
> 

Yes. It's useful in testing, but not so much in production. Changing it to debug.

>> +	}
>> +
>> +	if (int_status & CSI2_INT_PKT_FATAL) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT_FATAL);
>> +		pr_info_ratelimited("%08X CSI INT PKT FATAL: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +
>> +	if (int_status & CSI2_INT_FRAME_FATAL) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_FRAME_FATAL);
>> +		pr_info_ratelimited("%08X CSI INT FRAME FATAL: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +
>> +	if (int_status & CSI2_INT_PHY) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY);
>> +		pr_info_ratelimited("%08X CSI INT PHY: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +
>> +	if (int_status & CSI2_INT_PKT) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PKT);
>> +		pr_info_ratelimited("%08X CSI INT PKT: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +
>> +	if (int_status & CSI2_INT_LINE) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_LINE);
>> +		pr_info_ratelimited("%08X CSI INT LINE: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +
>> +	if (int_status & CSI2_INT_IPI) {
>> +		ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_IPI);
>> +		pr_info_ratelimited("%08X CSI INT IPI: %08X\n",
>> +				    (uint32_t) dev->base_address, ind_status);
>> +	}
>> +	spin_unlock_irqrestore(&dev->slock, flags);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int
>> +dw_mipi_csi_parse_dt(struct platform_device *pdev, struct mipi_csi_dev *dev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +	int reg;
>> +	int ret = 0;
>> +
>> +	/* Device tree information */
>> +	ret = of_property_read_u32(node, "data-lanes", &dev->hw.num_lanes);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read data-lanes\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = of_property_read_u32(node, "output-type", &dev->hw.output_type);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read output-type\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = of_property_read_u32(node, "ipi-mode", &dev->hw.ipi_mode);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read ipi-mode\n");
>> +		return ret;
>> +	}
>> +
>> +	ret =
>> +	    of_property_read_u32(node, "ipi-auto-flush",
>> +				 &dev->hw.ipi_auto_flush);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read ipi-auto-flush\n");
>> +		return ret;
>> +	}
>> +
>> +	ret =
>> +	    of_property_read_u32(node, "ipi-color-mode",
>> +				 &dev->hw.ipi_color_mode);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read ipi-color-mode\n");
>> +		return ret;
>> +	}
>> +
>> +	ret =
>> +	    of_property_read_u32(node, "virtual-channel", &dev->hw.virtual_ch);
> 
> Is this some thing that should come from DT? Sure, we don't really have
> proper support for virtual channels right now, but could you use zero for
> now?
> 

I'm not sure if I'm understanding wha't you're saying.

This info is used by one of our CSI-2 Host outputs to determine which virtual
channel info should be processed.

>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read virtual-channel\n");
>> +		return ret;
>> +	}
>> +
>> +	node = of_get_child_by_name(node, "port");
>> +	if (!node)
>> +		return -EINVAL;
>> +
>> +	ret = of_property_read_u32(node, "reg", &reg);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Couldn't read reg value\n");
>> +		return ret;
>> +	}
>> +	dev->index = reg - 1;
>> +
>> +	if (dev->index >= CSI_MAX_ENTITIES)
>> +		return -ENXIO;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id dw_mipi_csi_of_match[];
>> +
>> +/**
>> + * @short Initialization routine - Entry point of the driver
>> + * @param[in] pdev pointer to the platform device structure
>> + * @return 0 on success and a negative number on failure
>> + * Refer to Linux errors.
>> + */
>> +static int mipi_csi_probe(struct platform_device *pdev)
>> +{
>> +	const struct of_device_id *of_id;
>> +	struct device *dev = &pdev->dev;
>> +	struct resource *res = NULL;
>> +	struct mipi_csi_dev *mipi_csi;
>> +	int ret = -ENOMEM;
>> +
>> +	dev_info(&pdev->dev, "Installing MIPI CSI-2 module\n");
>> +
>> +	dev_dbg(&pdev->dev, "Device registration\n");
> 
> At least one of these is redundant. I'd just remove both.
> 

I'll remove them.

>> +	mipi_csi = devm_kzalloc(dev, sizeof(*mipi_csi), GFP_KERNEL);
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&mipi_csi->lock);
>> +	spin_lock_init(&mipi_csi->slock);
>> +	mipi_csi->pdev = pdev;
>> +
>> +	of_id = of_match_node(dw_mipi_csi_of_match, dev->of_node);
>> +	if (WARN_ON(of_id == NULL))
>> +		return -EINVAL;
>> +
>> +	ret = dw_mipi_csi_parse_dt(pdev, mipi_csi);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	dev_info(dev, "Request DPHY\n");
>> +	mipi_csi->phy = devm_phy_get(dev, "csi2-dphy");
>> +	if (IS_ERR(mipi_csi->phy))
>> +		return PTR_ERR(mipi_csi->phy);
> 
> It'd be better to tell what's wrong when there's an error instead of
> printing a series of messages even when all is well.
> 

You're right, I'll remove the prints regarding success and add some to inform of
errors.

>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	mipi_csi->base_address = devm_ioremap_resource(dev, res);
>> +
>> +	if (IS_ERR(mipi_csi->base_address))
>> +		return PTR_ERR(mipi_csi->base_address);
>> +
>> +	mipi_csi->ctrl_irq_number = platform_get_irq(pdev, 0);
>> +	if (mipi_csi->ctrl_irq_number <= 0) {
>> +		dev_err(dev, "IRQ number not set.\n");
>> +		return mipi_csi->ctrl_irq_number;
>> +	}
>> +
>> +	ret = devm_request_irq(dev, mipi_csi->ctrl_irq_number,
>> +			       dw_mipi_csi_irq1, IRQF_SHARED,
>> +			       dev_name(dev), mipi_csi);
>> +	if (ret) {
>> +		dev_err(dev, "IRQ failed\n");
>> +		goto end;
>> +	}
>> +
>> +	v4l2_subdev_init(&mipi_csi->sd, &dw_mipi_csi_subdev_ops);
>> +	mipi_csi->sd.owner = THIS_MODULE;
>> +	snprintf(mipi_csi->sd.name, sizeof(mipi_csi->sd.name), "%s.%d",
>> +		 CSI_DEVICE_NAME, mipi_csi->index);
>> +	mipi_csi->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +	mipi_csi->fmt = &dw_mipi_csi_formats[0];
>> +
>> +	mipi_csi->format.code = dw_mipi_csi_formats[0].code;
>> +	mipi_csi->format.width = MIN_WIDTH;
>> +	mipi_csi->format.height = MIN_HEIGHT;
>> +
>> +	mipi_csi->pads[CSI_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
>> +	mipi_csi->pads[CSI_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
>> +	ret = media_entity_pads_init(&mipi_csi->sd.entity,
>> +				     CSI_PADS_NUM, mipi_csi->pads);
>> +
>> +	if (ret < 0) {
>> +		dev_err(dev, "Media Entity init failed\n");
>> +		goto entity_cleanup;
>> +	}
>> +
>> +	/* This allows to retrieve the platform device id by the host driver */
>> +	v4l2_set_subdevdata(&mipi_csi->sd, pdev);
>> +
>> +	/* .. and a pointer to the subdev. */
>> +	platform_set_drvdata(pdev, &mipi_csi->sd);
>> +
>> +	dw_mipi_csi_mask_irq_power_off(mipi_csi);
>> +	dev_info(dev, "DW MIPI CSI-2 Host registered successfully\n");
>> +	return 0;
>> +
>> +entity_cleanup:
>> +	media_entity_cleanup(&mipi_csi->sd.entity);
>> +end:
>> +	return ret;
>> +}
>> +
>> +/**
>> + * @short Exit routine - Exit point of the driver
>> + * @param[in] pdev pointer to the platform device structure
>> + * @return 0 on success and a negative number on failure
>> + * Refer to Linux errors.
>> + */
>> +static int mipi_csi_remove(struct platform_device *pdev)
>> +{
>> +	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
>> +	struct mipi_csi_dev *mipi_csi = sd_to_mipi_csi_dev(sd);
>> +
>> +	dev_dbg(&pdev->dev, "Removing MIPI CSI-2 module\n");
>> +	media_entity_cleanup(&mipi_csi->sd.entity);
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * @short of_device_id structure
>> + */
>> +static const struct of_device_id dw_mipi_csi_of_match[] = {
>> +	{
>> +	 .compatible = "snps,dw-mipi-csi"},
>> +	{ /* sentinel */ },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, dw_mipi_csi_of_match);
>> +
>> +/**
>> + * @short Platform driver structure
>> + */
>> +static struct platform_driver __refdata dw_mipi_csi_pdrv = {
>> +	.remove = mipi_csi_remove,
>> +	.probe = mipi_csi_probe,
>> +	.driver = {
>> +		   .name = CSI_DEVICE_NAME,
>> +		   .owner = THIS_MODULE,
>> +		   .of_match_table = dw_mipi_csi_of_match,
>> +		   },
>> +};
>> +
>> +module_platform_driver(dw_mipi_csi_pdrv);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Synopsys DW MIPI CSI-2 Host driver");
>> diff --git a/drivers/media/platform/dwc/dw_mipi_csi.h b/drivers/media/platform/dwc/dw_mipi_csi.h
>> new file mode 100644
>> index 0000000..5e4c48c
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/dw_mipi_csi.h
>> @@ -0,0 +1,179 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. 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 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef DW_MIPI_CSI_H_
>> +#define DW_MIPI_CSI_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/delay.h>
>> +#include <linux/wait.h>
>> +#include <linux/string.h>
>> +#include <linux/phy/phy.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-dv-timings.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/io.h>
> 
> Please arrange alphabetically. Same for other files.
> 
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define CSI_DEVICE_NAME	"dw-mipi-csi"
>> +
>> +/** @short DWC MIPI CSI-2 register addresses*/
>> +enum register_addresses {
>> +	R_CSI2_VERSION = 0x00,
>> +	R_CSI2_N_LANES = 0x04,
>> +	R_CSI2_CTRL_RESETN = 0x08,
>> +	R_CSI2_INTERRUPT = 0x0C,
>> +	R_CSI2_DATA_IDS_1 = 0x10,
>> +	R_CSI2_DATA_IDS_2 = 0x14,
>> +	R_CSI2_IPI_MODE = 0x80,
>> +	R_CSI2_IPI_VCID = 0x84,
>> +	R_CSI2_IPI_DATA_TYPE = 0x88,
>> +	R_CSI2_IPI_MEM_FLUSH = 0x8C,
>> +	R_CSI2_IPI_HSA_TIME = 0x90,
>> +	R_CSI2_IPI_HBP_TIME = 0x94,
>> +	R_CSI2_IPI_HSD_TIME = 0x98,
>> +	R_CSI2_IPI_HLINE_TIME = 0x9C,
>> +	R_CSI2_IPI_VSA_LINES = 0xB0,
>> +	R_CSI2_IPI_VBP_LINES = 0xB4,
>> +	R_CSI2_IPI_VFP_LINES = 0xB8,
>> +	R_CSI2_IPI_VACTIVE_LINES = 0xBC,
>> +	R_CSI2_INT_PHY_FATAL = 0xe0,
>> +	R_CSI2_MASK_INT_PHY_FATAL = 0xe4,
>> +	R_CSI2_FORCE_INT_PHY_FATAL = 0xe8,
>> +	R_CSI2_INT_PKT_FATAL = 0xf0,
>> +	R_CSI2_MASK_INT_PKT_FATAL = 0xf4,
>> +	R_CSI2_FORCE_INT_PKT_FATAL = 0xf8,
>> +	R_CSI2_INT_FRAME_FATAL = 0x100,
>> +	R_CSI2_MASK_INT_FRAME_FATAL = 0x104,
>> +	R_CSI2_FORCE_INT_FRAME_FATAL = 0x108,
>> +	R_CSI2_INT_PHY = 0x110,
>> +	R_CSI2_MASK_INT_PHY = 0x114,
>> +	R_CSI2_FORCE_INT_PHY = 0x118,
>> +	R_CSI2_INT_PKT = 0x120,
>> +	R_CSI2_MASK_INT_PKT = 0x124,
>> +	R_CSI2_FORCE_INT_PKT = 0x128,
>> +	R_CSI2_INT_LINE = 0x130,
>> +	R_CSI2_MASK_INT_LINE = 0x134,
>> +	R_CSI2_FORCE_INT_LINE = 0x138,
>> +	R_CSI2_INT_IPI = 0x140,
>> +	R_CSI2_MASK_INT_IPI = 0x144,
>> +	R_CSI2_FORCE_INT_IPI = 0x148
>> +};
>> +
>> +/** @short IPI Data Types */
>> +enum data_type {
>> +	CSI_2_YUV420_8 = 0x18,
>> +	CSI_2_YUV420_10 = 0x19,
>> +	CSI_2_YUV420_8_LEG = 0x1A,
>> +	CSI_2_YUV420_8_SHIFT = 0x1C,
>> +	CSI_2_YUV420_10_SHIFT = 0x1D,
>> +	CSI_2_YUV422_8 = 0x1E,
>> +	CSI_2_YUV422_10 = 0x1F,
>> +	CSI_2_RGB444 = 0x20,
>> +	CSI_2_RGB555 = 0x21,
>> +	CSI_2_RGB565 = 0x22,
>> +	CSI_2_RGB666 = 0x23,
>> +	CSI_2_RGB888 = 0x24,
>> +	CSI_2_RAW6 = 0x28,
>> +	CSI_2_RAW7 = 0x29,
>> +	CSI_2_RAW8 = 0x2A,
>> +	CSI_2_RAW10 = 0x2B,
>> +	CSI_2_RAW12 = 0x2C,
>> +	CSI_2_RAW14 = 0x2D,
>> +};
>> +
>> +/** @short Interrupt Masks */
>> +enum interrupt_type {
>> +	CSI2_INT_PHY_FATAL = 1 << 0,
>> +	CSI2_INT_PKT_FATAL = 1 << 1,
>> +	CSI2_INT_FRAME_FATAL = 1 << 2,
>> +	CSI2_INT_PHY = 1 << 16,
>> +	CSI2_INT_PKT = 1 << 17,
>> +	CSI2_INT_LINE = 1 << 18,
>> +	CSI2_INT_IPI = 1 << 19,
>> +
>> +};
>> +
>> +/** @short DWC MIPI CSI-2 output types*/
>> +enum output_type {
>> +	IPI_OUT = 0,
>> +	IDI_OUT = 1,
>> +	BOTH_OUT = 2
>> +};
>> +
>> +/** @short IPI output types*/
>> +enum ipi_output_type {
>> +	CAMERA_TIMING = 0,
>> +	AUTO_TIMING = 1
>> +};
>> +
>> +/**
>> + * @short Format template
>> + */
>> +struct mipi_fmt {
>> +	const char *name;
>> +	u32 code;
>> +	u8 depth;
>> +};
>> +
>> +struct csi_hw {
>> +
>> +	uint32_t num_lanes;
>> +	uint32_t output_type;
>> +
>> +	/*IPI Info */
>> +	uint32_t ipi_mode;
>> +	uint32_t ipi_color_mode;
>> +	uint32_t ipi_auto_flush;
>> +	uint32_t virtual_ch;
>> +
>> +	uint32_t hsa;
>> +	uint32_t hbp;
>> +	uint32_t hsd;
>> +	uint32_t htotal;
>> +
>> +	uint32_t vsa;
>> +	uint32_t vbp;
>> +	uint32_t vfp;
>> +	uint32_t vactive;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct mipi_csi_dev {
>> +	struct v4l2_subdev sd;
>> +	struct video_device vdev;
>> +
>> +	struct mutex lock;
>> +	spinlock_t slock;
>> +	struct media_pad pads[CSI_PADS_NUM];
>> +	struct platform_device *pdev;
>> +	u8 index;
>> +
>> +	/** Store current format */
>> +	const struct mipi_fmt *fmt;
>> +	struct v4l2_mbus_framefmt format;
>> +
>> +	/** Device Tree Information */
>> +	void __iomem *base_address;
>> +	uint32_t ctrl_irq_number;
>> +
>> +	struct csi_hw hw;
>> +
>> +	struct phy *phy;
>> +};
>> +
>> +#endif				/* DW_MIPI_CSI */
>> diff --git a/drivers/media/platform/dwc/plat_ipk.c b/drivers/media/platform/dwc/plat_ipk.c
>> new file mode 100644
>> index 0000000..48b9e36
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk.c
>> @@ -0,0 +1,835 @@
>> +/**
>> + * DWC MIPI CSI-2 Host IPK platform device driver
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
>> + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "plat_ipk.h"
>> +#include "video_device.h"
>> +#include "dw_mipi_csi.h"
>> +#include <media/v4l2-of.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +static int
>> +__plat_ipk_pipeline_s_format(struct plat_ipk_media_pipeline *ep,
>> +			     struct v4l2_subdev_format *fmt)
>> +{
>> +
>> +	struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> +	static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
>> +
>> +	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
>> +	v4l2_subdev_call(p->subdevs[seq[IDX_CSI]], pad, set_fmt, NULL, fmt);
>> +
>> +	return 0;
>> +}
>> +
>> +static void
>> +plat_ipk_pipeline_prepare(struct plat_ipk_pipeline *p, struct media_entity *me)
>> +{
>> +	struct v4l2_subdev *sd;
>> +	int i = 0;
> 
> unsigned int?
> 
>> +
>> +	for (i = 0; i < IDX_MAX; i++)
>> +		p->subdevs[i] = NULL;
>> +
>> +	while (1) {
>> +		struct media_pad *pad = NULL;
>> +
>> +		for (i = 0; i < me->num_pads; i++) {
>> +			struct media_pad *spad = &me->pads[i];
>> +
>> +			if (!(spad->flags & MEDIA_PAD_FL_SINK))
>> +				continue;
>> +
>> +			pad = media_entity_remote_pad(spad);
>> +			if (pad)
>> +				break;
>> +		}
>> +		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>> +			break;
>> +
>> +		sd = media_entity_to_v4l2_subdev(pad->entity);
>> +
>> +		switch (sd->grp_id) {
>> +		case GRP_ID_SENSOR:
>> +			p->subdevs[IDX_SENSOR] = sd;
>> +			break;
>> +		case GRP_ID_CSI:
>> +			p->subdevs[IDX_CSI] = sd;
>> +			break;
>> +		case GRP_ID_VIDEODEV:
>> +			p->subdevs[IDX_VDEV] = sd;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +		me = &sd->entity;
>> +		if (me->num_pads == 1)
>> +			break;
>> +	}
>> +}
>> +
>> +static int __subdev_set_power(struct v4l2_subdev *sd, int on)
>> +{
>> +	int *use_count;
>> +	int ret;
>> +
>> +	if (sd == NULL) {
>> +		pr_info("null subdev\n");
>> +		return -ENXIO;
>> +	}
>> +	use_count = &sd->entity.use_count;
>> +	if (on && (*use_count)++ > 0)
>> +		return 0;
>> +	else if (!on && (*use_count == 0 || --(*use_count) > 0))
>> +		return 0;
>> +
>> +	pr_debug("%d %s !\n", on, sd->entity.name);
> 
> Please do use dev_*() family of functions instead.
> 

This print is not really useful in production, so I'll remove it.

>> +	ret = v4l2_subdev_call(sd, core, s_power, on);
>> +
>> +	return ret != -ENOIOCTLCMD ? ret : 0;
>> +}
>> +
>> +static int plat_ipk_pipeline_s_power(struct plat_ipk_pipeline *p, bool on)
>> +{
>> +	static const u8 seq[IDX_MAX] = {IDX_CSI, IDX_SENSOR, IDX_VDEV};
>> +	int i, ret = 0;
>> +
>> +	for (i = 0; i < IDX_MAX; i++) {
>> +		unsigned int idx = seq[i];
>> +
>> +		if (p->subdevs[idx] == NULL)
>> +			pr_info("No device registered on %d\n", idx);
>> +		else {
>> +			ret = __subdev_set_power(p->subdevs[idx], on);
>> +			if (ret < 0 && ret != -ENXIO)
>> +				goto error;
>> +		}
>> +	}
>> +	return 0;
>> +error:
>> +	for (; i >= 0; i--) {
>> +		unsigned int idx = seq[i];
>> +
>> +		__subdev_set_power(p->subdevs[idx], !on);
>> +	}
>> +	return ret;
>> +}
>> +
>> +static int
>> +__plat_ipk_pipeline_open(struct plat_ipk_media_pipeline *ep,
>> +			 struct media_entity *me, bool prepare)
>> +{
>> +	struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> +	int ret;
>> +
>> +	if (WARN_ON(p == NULL || me == NULL))
>> +		return -EINVAL;
>> +
>> +	if (prepare)
>> +		plat_ipk_pipeline_prepare(p, me);
>> +
>> +	ret = plat_ipk_pipeline_s_power(p, 1);
>> +	if (!ret)
>> +		return 0;
>> +
>> +	return ret;
>> +}
>> +
>> +static int __plat_ipk_pipeline_close(struct plat_ipk_media_pipeline *ep)
>> +{
>> +	struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> +	int ret;
>> +
>> +	ret = plat_ipk_pipeline_s_power(p, 0);
>> +
>> +	return ret == -ENXIO ? 0 : ret;
>> +}
>> +
>> +static int
>> +__plat_ipk_pipeline_s_stream(struct plat_ipk_media_pipeline *ep, bool on)
>> +{
>> +	static const u8 seq[IDX_MAX] = {IDX_SENSOR, IDX_CSI, IDX_VDEV};
>> +	struct plat_ipk_pipeline *p = to_plat_ipk_pipeline(ep);
>> +	int i, ret = 0;
>> +
>> +	for (i = 0; i < IDX_MAX; i++) {
>> +		unsigned int idx = seq[i];
>> +
>> +		if (p->subdevs[idx] == NULL)
>> +			pr_debug("No device registered on %d\n", idx);
>> +		else {
>> +			ret =
>> +			    v4l2_subdev_call(p->subdevs[idx], video, s_stream,
>> +					     on);
>> +
>> +			if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> +				goto error;
>> +		}
>> +	}
>> +	return 0;
>> +error:
>> +	for (; i >= 0; i--) {
>> +		unsigned int idx = seq[i];
>> +
>> +		v4l2_subdev_call(p->subdevs[idx], video, s_stream, !on);
>> +	}
>> +	return ret;
>> +}
>> +
>> +static const struct plat_ipk_media_pipeline_ops plat_ipk_pipeline_ops = {
>> +	.open = __plat_ipk_pipeline_open,
>> +	.close = __plat_ipk_pipeline_close,
>> +	.set_format = __plat_ipk_pipeline_s_format,
>> +	.set_stream = __plat_ipk_pipeline_s_stream,
>> +};
>> +
>> +static struct plat_ipk_media_pipeline *
>> +plat_ipk_pipeline_create(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	struct plat_ipk_pipeline *p;
>> +
>> +	p = kzalloc(sizeof(*p), GFP_KERNEL);
>> +	if (!p)
>> +		return NULL;
>> +
>> +	list_add_tail(&p->list, &plat_ipk->pipelines);
>> +
>> +	p->ep.ops = &plat_ipk_pipeline_ops;
>> +	return &p->ep;
>> +}
>> +
>> +static void
>> +plat_ipk_pipelines_free(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	while (!list_empty(&plat_ipk->pipelines)) {
>> +		struct plat_ipk_pipeline *p;
>> +
>> +		p = list_entry(plat_ipk->pipelines.next, typeof(*p), list);
>> +		list_del(&p->list);
>> +		kfree(p);
>> +	}
>> +}
>> +
>> +static int
>> +plat_ipk_parse_port_node(struct plat_ipk_dev *plat_ipk,
>> +			 struct device_node *port, unsigned int index)
>> +{
>> +	struct device_node *rem, *ep;
>> +	struct v4l2_of_endpoint endpoint;
>> +	struct plat_ipk_source_info *pd = &plat_ipk->sensor[index].pdata;
>> +
>> +	/* Assume here a port node can have only one endpoint node. */
>> +	ep = of_get_next_child(port, NULL);
>> +	if (!ep)
>> +		return 0;
>> +
>> +	v4l2_of_parse_endpoint(ep, &endpoint);
>> +	if (WARN_ON(endpoint.base.port == 0) || index >= PLAT_MAX_SENSORS)
>> +		return -EINVAL;
>> +
>> +	pd->mux_id = endpoint.base.port - 1;
>> +
>> +	rem = of_graph_get_remote_port_parent(ep);
>> +	of_node_put(ep);
>> +	if (rem == NULL) {
>> +		v4l2_info(&plat_ipk->v4l2_dev,
>> +			  "Remote device at %s not found\n", ep->full_name);
>> +		return 0;
>> +	}
>> +
>> +	if (WARN_ON(index >= ARRAY_SIZE(plat_ipk->sensor)))
>> +		return -EINVAL;
>> +
>> +	plat_ipk->sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF;
>> +	plat_ipk->sensor[index].asd.match.of.node = rem;
>> +	plat_ipk->async_subdevs[index] = &plat_ipk->sensor[index].asd;
>> +
>> +	plat_ipk->num_sensors++;
>> +
>> +	of_node_put(rem);
>> +	return 0;
>> +}
>> +
>> +
>> +static int plat_ipk_register_sensor_entities(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	struct device_node *parent = plat_ipk->pdev->dev.of_node;
>> +	struct device_node *node;
>> +	int index = 0;
>> +	int ret;
>> +
>> +	plat_ipk->num_sensors = 0;
>> +
>> +	for_each_available_child_of_node(parent, node) {
>> +		struct device_node *port;
>> +
>> +		if (of_node_cmp(node->name, "csi2"))
>> +			continue;
>> +		port = of_get_next_child(node, NULL);
>> +		if (!port)
>> +			continue;
>> +
>> +		ret = plat_ipk_parse_port_node(plat_ipk, port, index);
>> +		if (ret < 0)
>> +			return ret;
>> +		index++;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int
>> +__of_get_port_id(struct device_node *np)
>> +{
>> +	u32 reg = 0;
>> +
>> +	np = of_get_child_by_name(np, "port");
>> +	if (!np)
>> +		return -EINVAL;
>> +	of_property_read_u32(np, "reg", &reg);
>> +
>> +	return reg - 1;
>> +}
>> +
>> +static int register_videodev_entity(struct plat_ipk_dev *plat_ipk,
>> +			 struct video_device_dev *vid_dev)
>> +{
>> +	struct v4l2_subdev *sd;
>> +	struct plat_ipk_media_pipeline *ep;
>> +	int ret;
>> +
>> +	sd = &vid_dev->subdev;
>> +	sd->grp_id = GRP_ID_VIDEODEV;
>> +
>> +	ep = plat_ipk_pipeline_create(plat_ipk);
>> +	if (!ep)
>> +		return -ENOMEM;
>> +
>> +	v4l2_set_subdev_hostdata(sd, ep);
>> +
>> +	ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
>> +	if (!ret)
>> +		plat_ipk->vid_dev = vid_dev;
>> +	else
>> +		v4l2_err(&plat_ipk->v4l2_dev,
>> +			 "Failed to register Video Device\n");
>> +	return ret;
>> +}
>> +
>> +static int register_mipi_csi_entity(struct plat_ipk_dev *plat_ipk,
>> +			 struct platform_device *pdev, struct v4l2_subdev *sd)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +	int id, ret;
>> +
>> +	id = node ? __of_get_port_id(node) : max(0, pdev->id);
>> +
>> +	if (WARN_ON(id < 0 || id >= CSI_MAX_ENTITIES))
>> +		return -ENOENT;
>> +
>> +	if (WARN_ON(plat_ipk->mipi_csi[id].sd))
>> +		return -EBUSY;
>> +
>> +	sd->grp_id = GRP_ID_CSI;
>> +	ret = v4l2_device_register_subdev(&plat_ipk->v4l2_dev, sd);
>> +
>> +	if (!ret)
>> +		plat_ipk->mipi_csi[id].sd = sd;
>> +	else
>> +		v4l2_err(&plat_ipk->v4l2_dev,
>> +			 "Failed to register MIPI-CSI.%d (%d)\n", id, ret);
>> +	return ret;
>> +}
>> +
>> +static int plat_ipk_register_platform_entity(struct plat_ipk_dev *plat_ipk,
>> +				struct platform_device *pdev, int plat_entity)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int ret = -EPROBE_DEFER;
>> +	void *drvdata;
>> +
>> +
> 
> Extra newline.
> 

Removing it.

>> +	device_lock(dev);
>> +	if (!dev->driver || !try_module_get(dev->driver->owner))
>> +		goto dev_unlock;
>> +
>> +	drvdata = dev_get_drvdata(dev);
>> +
>> +	if (drvdata) {
>> +		switch (plat_entity) {
>> +		case IDX_VDEV:
>> +			ret = register_videodev_entity(plat_ipk, drvdata);
>> +			break;
>> +		case IDX_CSI:
>> +			ret = register_mipi_csi_entity(plat_ipk, pdev, drvdata);
>> +			break;
>> +		default:
>> +			ret = -ENODEV;
>> +		}
>> +	} else
>> +		dev_err(&plat_ipk->pdev->dev, "%s no drvdata\n", dev_name(dev));
>> +	module_put(dev->driver->owner);
>> +dev_unlock:
>> +	device_unlock(dev);
>> +	if (ret == -EPROBE_DEFER)
>> +		dev_info(&plat_ipk->pdev->dev,
>> +			 "deferring %s device registration\n", dev_name(dev));
>> +	else if (ret < 0)
>> +		dev_err(&plat_ipk->pdev->dev,
>> +			"%s device registration failed (%d)\n", dev_name(dev),
>> +			ret);
>> +	return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_register_platform_entities(struct plat_ipk_dev *plat_ipk,
>> +				    struct device_node *parent)
>> +{
>> +	struct device_node *node;
>> +	int ret = 0;
>> +
>> +	for_each_available_child_of_node(parent, node) {
>> +		struct platform_device *pdev;
>> +		int plat_entity = -1;
>> +
>> +		pdev = of_find_device_by_node(node);
>> +		if (!pdev)
>> +			continue;
>> +
>> +		if (!strcmp(node->name, VIDEODEV_OF_NODE_NAME))
>> +			plat_entity = IDX_VDEV;
>> +		else if (!strcmp(node->name, CSI_OF_NODE_NAME))
>> +			plat_entity = IDX_CSI;
>> +
>> +		if (plat_entity >= 0)
>> +			ret = plat_ipk_register_platform_entity(plat_ipk, pdev,
>> +								plat_entity);
>> +		put_device(&pdev->dev);
>> +		if (ret < 0)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static void
>> +plat_ipk_unregister_entities(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	int i;
>> +	struct video_device_dev *dev = plat_ipk->vid_dev;
>> +
>> +	if (dev == NULL)
>> +		return;
>> +	v4l2_device_unregister_subdev(&dev->subdev);
>> +	dev->ve.pipe = NULL;
>> +	plat_ipk->vid_dev = NULL;
>> +
>> +	for (i = 0; i < CSI_MAX_ENTITIES; i++) {
>> +		if (plat_ipk->mipi_csi[i].sd == NULL)
>> +			continue;
>> +		v4l2_device_unregister_subdev(plat_ipk->mipi_csi[i].sd);
>> +		plat_ipk->mipi_csi[i].sd = NULL;
>> +	}
>> +
>> +	v4l2_info(&plat_ipk->v4l2_dev, "Unregistered all entities\n");
>> +}
>> +
>> +static int
>> +__plat_ipk_create_videodev_sink_links(struct plat_ipk_dev *plat_ipk,
>> +				      struct media_entity *source,
>> +				      int pad)
>> +{
>> +	struct media_entity *sink;
>> +	int ret = 0;
>> +
>> +	if (!plat_ipk->vid_dev)
>> +		return 0;
>> +
>> +	sink = &plat_ipk->vid_dev->subdev.entity;
>> +	ret = media_create_pad_link(source, pad, sink,
>> +				    CSI_PAD_SOURCE, MEDIA_LNK_FL_ENABLED);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = media_entity_call(sink, link_setup, &sink->pads[0],
>> +				&source->pads[pad], 0);
> 
> What's the purpose of calling link_setup() here?
> 

None, this was useful in the past but it's not being used right now. I'll remove it.

>> +	if (ret)
>> +		return 0;
>> +
>> +	v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> +		  source->name, sink->name);
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static int
>> +__plat_ipk_create_videodev_source_links(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	struct media_entity *source, *sink;
>> +	int ret = 0;
>> +
>> +	struct video_device_dev *vid_dev = plat_ipk->vid_dev;
>> +
>> +	if (vid_dev == NULL)
>> +		return -ENODEV;
>> +
>> +	source = &vid_dev->subdev.entity;
>> +	sink = &vid_dev->ve.vdev.entity;
>> +
>> +	ret = media_create_pad_link(source, VIDEO_DEV_SD_PAD_SOURCE_DMA,
>> +				    sink, 0, MEDIA_LNK_FL_ENABLED);
>> +
>> +	v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> +		  source->name, sink->name);
>> +	return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_create_links(struct plat_ipk_dev *plat_ipk)
>> +{
>> +	struct v4l2_subdev *csi_sensor[CSI_MAX_ENTITIES] = { NULL };
>> +	struct v4l2_subdev *sensor, *csi;
>> +	struct media_entity *source;
>> +	struct plat_ipk_source_info *pdata;
>> +	int i, pad, ret = 0;
>> +
>> +	for (i = 0; i < plat_ipk->num_sensors; i++) {
>> +		if (plat_ipk->sensor[i].subdev == NULL)
>> +			continue;
>> +
>> +		sensor = plat_ipk->sensor[i].subdev;
>> +		pdata = v4l2_get_subdev_hostdata(sensor);
>> +		if (!pdata)
>> +			continue;
>> +
>> +		source = NULL;
>> +
>> +		csi = plat_ipk->mipi_csi[pdata->mux_id].sd;
>> +		if (WARN(csi == NULL, "MIPI-CSI interface specified but	dw-mipi-csi module is not loaded!\n"))
>> +			return -EINVAL;
>> +
>> +		pad = sensor->entity.num_pads - 1;
>> +		ret = media_create_pad_link(&sensor->entity, pad,
>> +					    &csi->entity, CSI_PAD_SINK,
>> +					    MEDIA_LNK_FL_IMMUTABLE |
>> +					    MEDIA_LNK_FL_ENABLED);
>> +
>> +		if (ret)
>> +			return ret;
>> +		v4l2_info(&plat_ipk->v4l2_dev, "created link [%s] -> [%s]\n",
>> +			  sensor->entity.name, csi->entity.name);
> 
> dev_dbg() ?
> 

Yes. Changing it.

>> +
>> +		csi_sensor[pdata->mux_id] = sensor;
>> +	}
>> +
>> +	for (i = 0; i < CSI_MAX_ENTITIES; i++) {
>> +		if (plat_ipk->mipi_csi[i].sd == NULL) {
>> +			pr_info("no link\n");
>> +			continue;
>> +		}
>> +
>> +		source = &plat_ipk->mipi_csi[i].sd->entity;
>> +		pad = VIDEO_DEV_SD_PAD_SINK_CSI;
>> +
>> +		ret = __plat_ipk_create_videodev_sink_links(plat_ipk, source,
>> +								pad);
>> +	}
>> +
>> +	ret = __plat_ipk_create_videodev_source_links(plat_ipk);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return ret;
>> +}
>> +
>> +static int __plat_ipk_modify_pipeline(struct media_entity *entity, bool enable)
>> +{
>> +	struct plat_ipk_video_entity *ve;
>> +	struct plat_ipk_pipeline *p;
>> +	struct video_device *vdev;
>> +	int ret;
>> +
>> +	vdev = media_entity_to_video_device(entity);
>> +
>> +	if (vdev->entity.use_count == 0)
>> +		return 0;
>> +
>> +	ve = vdev_to_plat_ipk_video_entity(vdev);
>> +	p = to_plat_ipk_pipeline(ve->pipe);
>> +
>> +	if (enable)
>> +		ret = __plat_ipk_pipeline_open(ve->pipe, entity, true);
>> +	else
>> +		ret = __plat_ipk_pipeline_close(ve->pipe);
>> +
>> +	if (ret == 0 && !enable)
>> +		memset(p->subdevs, 0, sizeof(p->subdevs));
>> +
>> +	return ret;
>> +}
>> +
>> +
>> +static int
>> +__plat_ipk_modify_pipelines(struct media_entity *entity, bool enable,
>> +			    struct media_entity_graph *graph)
>> +{
>> +	struct media_entity *entity_err = entity;
>> +	int ret;
>> +
>> +	media_entity_graph_walk_start(graph, entity);
>> +
>> +	while ((entity = media_entity_graph_walk_next(graph))) {
>> +		if (!is_media_entity_v4l2_video_device(entity))
>> +			continue;
>> +
>> +		ret = __plat_ipk_modify_pipeline(entity, enable);
>> +
>> +		if (ret < 0)
>> +			goto err;
>> +	}
>> +
>> +	return 0;
>> +
>> +err:
>> +	media_entity_graph_walk_start(graph, entity_err);
>> +
>> +	while ((entity_err = media_entity_graph_walk_next(graph))) {
>> +		if (!is_media_entity_v4l2_video_device(entity_err))
>> +			continue;
>> +
>> +		__plat_ipk_modify_pipeline(entity_err, !enable);
>> +
>> +		if (entity_err == entity)
>> +			break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int
>> +plat_ipk_link_notify(struct media_link *link, unsigned int flags,
>> +		     unsigned int notification)
>> +{
>> +	struct media_entity_graph *graph =
>> +	    &container_of(link->graph_obj.mdev, struct plat_ipk_dev,
>> +			  media_dev)->link_setup_graph;
>> +	struct media_entity *sink = link->sink->entity;
>> +	int ret = 0;
>> +
>> +	pr_debug("Link notify\n");
> 
> This probably made sense at development time but should be removed now.
> 

Yes. You're right.

>> +
>> +	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH) {
>> +		ret = media_entity_graph_walk_init(graph, link->graph_obj.mdev);
>> +		if (ret)
>> +			return ret;
>> +		if (!(flags & MEDIA_LNK_FL_ENABLED))
>> +			ret = __plat_ipk_modify_pipelines(sink, false, graph);
>> +
>> +	} else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH) {
>> +		if (link->flags & MEDIA_LNK_FL_ENABLED)
>> +			ret = __plat_ipk_modify_pipelines(sink, true, graph);
>> +		media_entity_graph_walk_cleanup(graph);
>> +	}
>> +
>> +	return ret ? -EPIPE : 0;
>> +}
> 
> Missing newline.
> 

Adding one.

>> +static const struct media_device_ops plat_ipk_media_ops = {
>> +	.link_notify = plat_ipk_link_notify,
>> +};
>> +
>> +
>> +static int
>> +subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>> +		      struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd)
>> +{
>> +	struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
>> +	struct plat_ipk_sensor_info *si = NULL;
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(plat_ipk->sensor); i++)
>> +		if (plat_ipk->sensor[i].asd.match.of.node ==
>> +		    subdev->dev->of_node)
>> +			si = &plat_ipk->sensor[i];
>> +
>> +	if (si == NULL)
>> +		return -EINVAL;
>> +
>> +	v4l2_set_subdev_hostdata(subdev, &si->pdata);
>> +
>> +	subdev->grp_id = GRP_ID_SENSOR;
>> +
>> +	si->subdev = subdev;
>> +
>> +	v4l2_info(&plat_ipk->v4l2_dev, "Registered sensor subdevice: %s (%d)\n",
>> +		  subdev->name, plat_ipk->num_sensors);
>> +
>> +	plat_ipk->num_sensors++;
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +subdev_notifier_complete(struct v4l2_async_notifier *notifier)
>> +{
>> +	struct plat_ipk_dev *plat_ipk = notifier_to_plat_ipk(notifier);
>> +	int ret;
>> +
>> +	mutex_lock(&plat_ipk->media_dev.graph_mutex);
>> +
>> +	ret = plat_ipk_create_links(plat_ipk);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	ret = v4l2_device_register_subdev_nodes(&plat_ipk->v4l2_dev);
>> +unlock:
>> +	mutex_unlock(&plat_ipk->media_dev.graph_mutex);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return media_device_register(&plat_ipk->media_dev);
>> +}
>> +
>> +static const struct of_device_id plat_ipk_of_match[];
> 
> This is redundant.
> 

You're right.

>> +
>> +static int plat_ipk_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct v4l2_device *v4l2_dev;
>> +	struct plat_ipk_dev *plat_ipk;
>> +	int ret;
>> +
>> +	dev_info(dev, "Installing DW MIPI CSI-2 IPK Platform module\n");
>> +
>> +	plat_ipk = devm_kzalloc(dev, sizeof(*plat_ipk), GFP_KERNEL);
>> +	if (!plat_ipk)
>> +		return -ENOMEM;
>> +
>> +	spin_lock_init(&plat_ipk->slock);
>> +	INIT_LIST_HEAD(&plat_ipk->pipelines);
>> +	plat_ipk->pdev = pdev;
>> +
>> +	strlcpy(plat_ipk->media_dev.model, "SNPS IPK Platform",
>> +		sizeof(plat_ipk->media_dev.model));
>> +	plat_ipk->media_dev.ops = &plat_ipk_media_ops;
>> +	plat_ipk->media_dev.dev = dev;
>> +
>> +	v4l2_dev = &plat_ipk->v4l2_dev;
>> +	v4l2_dev->mdev = &plat_ipk->media_dev;
>> +	strlcpy(v4l2_dev->name, "plat-ipk", sizeof(v4l2_dev->name));
>> +
>> +	media_device_init(&plat_ipk->media_dev);
>> +
>> +	ret = v4l2_device_register(dev, &plat_ipk->v4l2_dev);
>> +	if (ret < 0) {
>> +		v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, plat_ipk);
>> +
>> +	ret = plat_ipk_register_platform_entities(plat_ipk, dev->of_node);
>> +	if (ret)
>> +		goto err_m_ent;
>> +
>> +	ret = plat_ipk_register_sensor_entities(plat_ipk);
>> +	if (ret)
>> +		goto err_m_ent;
>> +
>> +	if (plat_ipk->num_sensors > 0) {
>> +		plat_ipk->subdev_notifier.subdevs = plat_ipk->async_subdevs;
>> +		plat_ipk->subdev_notifier.num_subdevs = plat_ipk->num_sensors;
>> +		plat_ipk->subdev_notifier.bound = subdev_notifier_bound;
>> +		plat_ipk->subdev_notifier.complete = subdev_notifier_complete;
>> +		plat_ipk->num_sensors = 0;
>> +
>> +		ret = v4l2_async_notifier_register(&plat_ipk->v4l2_dev,
>> +						   &plat_ipk->subdev_notifier);
>> +		if (ret)
>> +			goto err_m_ent;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_m_ent:
>> +	plat_ipk_unregister_entities(plat_ipk);
>> +	media_device_unregister(&plat_ipk->media_dev);
>> +	media_device_cleanup(&plat_ipk->media_dev);
>> +	v4l2_device_unregister(&plat_ipk->v4l2_dev);
>> +	return ret;
>> +}
>> +
>> +static int plat_ipk_remove(struct platform_device *pdev)
>> +{
>> +	struct plat_ipk_dev *dev = platform_get_drvdata(pdev);
>> +
>> +	if (!dev)
>> +		return 0;
> 
> Wouldn't !dev here require a driver bug? If probe() succeeds, then dev is
> non-zero, right?
> 

It was just a better safe than sorry situation, but if you think it's not
necessary I'll remove it.

>> +
>> +	v4l2_async_notifier_unregister(&dev->subdev_notifier);
>> +
>> +	v4l2_device_unregister(&dev->v4l2_dev);
>> +	plat_ipk_unregister_entities(dev);
>> +	plat_ipk_pipelines_free(dev);
>> +	media_device_unregister(&dev->media_dev);
>> +	media_device_cleanup(&dev->media_dev);
>> +
>> +	dev_info(&pdev->dev, "Driver removed\n");
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * @short of_device_id structure
>> + */
>> +static const struct of_device_id plat_ipk_of_match[] = {
>> +	{.compatible = "snps,plat-ipk"},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, plat_ipk_of_match);
>> +
>> +/**
>> + * @short Platform driver structure
>> + */
>> +static struct platform_driver plat_ipk_pdrv = {
>> +	.remove = plat_ipk_remove,
>> +	.probe = plat_ipk_probe,
>> +	.driver = {
>> +		   .name = "snps,plat-ipk",
>> +		   .owner = THIS_MODULE,
>> +		   .of_match_table = plat_ipk_of_match,
>> +		   },
>> +};
>> +
>> +static int __init
>> +plat_ipk_init(void)
>> +{
>> +	request_module("dw-mipi-csi");
> 
> Why do you need this?
> 

To make sure that module is loaded when I start loading this one.

Do you think it's not necessary to do this, or is there a better way to do this?

>> +
>> +	return platform_driver_register(&plat_ipk_pdrv);
>> +}
>> +
>> +static void __exit
>> +plat_ipk_exit(void)
>> +{
>> +	platform_driver_unregister(&plat_ipk_pdrv);
>> +}
>> +
>> +module_init(plat_ipk_init);
>> +module_exit(plat_ipk_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Platform driver for MIPI CSI-2 Host IPK");
>> diff --git a/drivers/media/platform/dwc/plat_ipk.h b/drivers/media/platform/dwc/plat_ipk.h
>> new file mode 100644
>> index 0000000..ef569eb
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. 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 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef PLAT_IPK_H_
>> +#define PLAT_IPK_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/list.h>
>> +#include <linux/string.h>
>> +#include <media/v4l2-device.h>
>> +#include <linux/videodev2.h>
>> +#include <media/media-entity.h>
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define VIDEODEV_OF_NODE_NAME	"video-device"
>> +#define CSI_OF_NODE_NAME	"csi2"
>> +
>> +enum plat_ipk_subdev_index {
>> +	IDX_SENSOR,
>> +	IDX_CSI,
>> +	IDX_VDEV,
>> +	IDX_MAX,
>> +};
>> +
>> +struct plat_ipk_sensor_info {
>> +	struct plat_ipk_source_info pdata;
>> +	struct v4l2_async_subdev asd;
>> +	struct v4l2_subdev *subdev;
>> +	struct mipi_csi_dev *host;
>> +};
>> +
>> +struct plat_ipk_pipeline {
>> +	struct plat_ipk_media_pipeline ep;
>> +	struct list_head list;
>> +	struct media_entity *vdev_entity;
>> +	struct v4l2_subdev *subdevs[IDX_MAX];
>> +};
>> +
>> +#define to_plat_ipk_pipeline(_ep)\
>> +	 container_of(_ep, struct plat_ipk_pipeline, ep)
>> +
>> +struct mipi_csi_info {
>> +	struct v4l2_subdev *sd;
>> +	int id;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct plat_ipk_dev {
>> +	struct mipi_csi_info		mipi_csi[CSI_MAX_ENTITIES];
>> +	struct video_device_dev		*vid_dev;
>> +	struct device			*dev;
>> +	struct media_device		media_dev;
>> +	struct v4l2_device		v4l2_dev;
>> +	struct platform_device		*pdev;
>> +	struct plat_ipk_sensor_info	sensor[PLAT_MAX_SENSORS];
>> +	struct v4l2_async_notifier	subdev_notifier;
>> +	struct v4l2_async_subdev	*async_subdevs[PLAT_MAX_SENSORS];
>> +	spinlock_t			slock;
>> +	struct list_head		pipelines;
>> +	int				num_sensors;
>> +	struct media_entity_graph	link_setup_graph;
>> +};
>> +
>> +static inline struct plat_ipk_dev *
>> +entity_to_plat_ipk_mdev(struct media_entity *me)
>> +{
>> +	return me->graph_obj.mdev == NULL ? NULL :
>> +	    container_of(me->graph_obj.mdev, struct plat_ipk_dev, media_dev);
>> +}
>> +
>> +static inline struct plat_ipk_dev *
>> +notifier_to_plat_ipk(struct v4l2_async_notifier *n)
>> +{
>> +	return container_of(n, struct plat_ipk_dev, subdev_notifier);
>> +}
>> +
>> +static inline void
>> +plat_ipk_graph_unlock(struct plat_ipk_video_entity *ve)
>> +{
>> +	mutex_unlock(&ve->vdev.entity.graph_obj.mdev->graph_mutex);
>> +}
>> +
>> +#endif				/* PLAT_IPK_H_ */
>> diff --git a/drivers/media/platform/dwc/plat_ipk_video.h b/drivers/media/platform/dwc/plat_ipk_video.h
>> new file mode 100644
>> index 0000000..6bfc9f8
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/plat_ipk_video.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. 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 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef PLAT_IPK_INCLUDES_H_
>> +#define PLAT_IPK_INCLUDES_H_
>> +
>> +#include <media/media-entity.h>
>> +#include <media/v4l2-dev.h>
>> +#include <media/v4l2-mediabus.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +/*
>> + * The subdevices' group IDs.
>> + */
>> +
>> +#define MAX_WIDTH	3280
>> +#define MAX_HEIGHT	1852
>> +
>> +#define MIN_WIDTH	640
>> +#define MIN_HEIGHT	480
>> +
>> +#define GRP_ID_SENSOR		(10)
>> +#define GRP_ID_CSI		(20)
>> +#define GRP_ID_VIDEODEV		(30)
>> +
>> +#define CSI_MAX_ENTITIES	1
>> +#define PLAT_MAX_SENSORS	1
>> +
>> +enum video_dev_pads {
>> +	VIDEO_DEV_SD_PAD_SINK_CSI = 0,
>> +	VIDEO_DEV_SD_PAD_SOURCE_DMA = 1,
>> +	VIDEO_DEV_SD_PADS_NUM = 2,
>> +};
>> +
>> +enum mipi_csi_pads {
>> +	CSI_PAD_SINK = 0,
>> +	CSI_PAD_SOURCE = 1,
>> +	CSI_PADS_NUM = 2,
>> +};
>> +
>> +struct plat_ipk_source_info {
>> +	u16 flags;
>> +	u16 mux_id;
>> +};
>> +
>> +struct plat_ipk_fmt {
>> +	u32 mbus_code;
>> +	char *name;
>> +	u32 fourcc;
>> +	u8 depth;
>> +};
>> +
>> +struct plat_ipk_media_pipeline;
>> +
>> +/*
>> + * Media pipeline operations to be called from within a video node,  i.e. the
>> + * last entity within the pipeline. Implemented by related media device driver.
>> + */
>> +struct plat_ipk_media_pipeline_ops {
>> +	int (*prepare)(struct plat_ipk_media_pipeline *p,
>> +			struct media_entity *me);
>> +	int (*unprepare)(struct plat_ipk_media_pipeline *p);
>> +	int (*open)(struct plat_ipk_media_pipeline *p,
>> +			struct media_entity *me, bool resume);
>> +	int (*close)(struct plat_ipk_media_pipeline *p);
>> +	int (*set_stream)(struct plat_ipk_media_pipeline *p, bool state);
>> +	int (*set_format)(struct plat_ipk_media_pipeline *p,
>> +			struct v4l2_subdev_format *fmt);
>> +};
>> +
>> +struct plat_ipk_video_entity {
>> +	struct video_device vdev;
>> +	struct plat_ipk_media_pipeline *pipe;
>> +};
>> +
>> +struct plat_ipk_media_pipeline {
>> +	struct media_pipeline mp;
>> +	const struct plat_ipk_media_pipeline_ops *ops;
>> +};
>> +
>> +static inline struct plat_ipk_video_entity *
>> +vdev_to_plat_ipk_video_entity(struct video_device *vdev)
>> +{
>> +	return container_of(vdev, struct plat_ipk_video_entity, vdev);
>> +}
>> +
>> +#define plat_ipk_pipeline_call(ent, op, args...)\
>> +	(!(ent) ? -ENOENT : (((ent)->pipe->ops && (ent)->pipe->ops->op) ? \
>> +	(ent)->pipe->ops->op(((ent)->pipe), ##args) : -ENOIOCTLCMD))	  \
>> +
>> +
>> +#endif				/* PLAT_IPK_INCLUDES_H_ */
>> diff --git a/drivers/media/platform/dwc/video_device.c b/drivers/media/platform/dwc/video_device.c
>> new file mode 100644
>> index 0000000..d827426
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/video_device.c
>> @@ -0,0 +1,741 @@
>> +/*
>> + * DWC MIPI CSI-2 Host IPK video device device driver
>> + *
>> + * Copyright (C) 2016 Synopsys, Inc. All rights reserved.
>> + * Author: Ramiro Oliveira <ramiro.oliveira@synopsys.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published
>> + * by the Free Software Foundation, either version 2 of the License,
>> + * or (at your option) any later version.
>> + */
>> +
>> +#include "video_device.h"
>> +
>> +const struct plat_ipk_fmt vid_dev_formats[] = {
>> +	{
>> +		.name = "RGB888",
>> +		.fourcc = V4L2_PIX_FMT_BGR24,
>> +		.depth = 24,
>> +		.mbus_code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>> +	}, {
>> +		.name = "RGB565",
>> +		.fourcc = V4L2_PIX_FMT_RGB565,
>> +		.depth = 16,
>> +		.mbus_code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>> +	},
>> +};
>> +
>> +const struct plat_ipk_fmt *
>> +vid_dev_find_format(struct v4l2_format *f, int index)
>> +{
>> +	const struct plat_ipk_fmt *fmt = NULL;
>> +	unsigned int i;
>> +
>> +	if (index >= (int) ARRAY_SIZE(vid_dev_formats))
>> +		return NULL;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(vid_dev_formats); ++i) {
>> +		fmt = &vid_dev_formats[i];
>> +		if (fmt->fourcc == f->fmt.pix.pixelformat)
>> +			return fmt;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * Video node ioctl operations
>> + */
>> +static int
>> +vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap)
>> +{
>> +	struct video_device_dev *vid_dev = video_drvdata(file);
>> +
>> +	strlcpy(cap->driver, VIDEO_DEVICE_NAME, sizeof(cap->driver));
>> +	strlcpy(cap->card, VIDEO_DEVICE_NAME, sizeof(cap->card));
>> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
>> +		 dev_name(&vid_dev->pdev->dev));
>> +
>> +	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
>> +	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
>> +	return 0;
>> +}
>> +
>> +int
>> +vidioc_enum_fmt_vid_cap(struct file *file, void *priv, struct v4l2_fmtdesc *f)
>> +{
>> +	const struct plat_ipk_fmt *p_fmt;
>> +
>> +	if (f->index >= ARRAY_SIZE(vid_dev_formats))
>> +		return -EINVAL;
>> +
>> +	p_fmt = &vid_dev_formats[f->index];
>> +
>> +	strlcpy(f->description, p_fmt->name, sizeof(f->description));
>> +	f->pixelformat = p_fmt->fourcc;
>> +
>> +	return 0;
>> +}
>> +
>> +int vidioc_g_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> +	struct video_device_dev *dev = video_drvdata(file);
>> +
>> +	memcpy(&f->fmt.pix, &dev->format.fmt.pix,
>> +	       sizeof(struct v4l2_pix_format));
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> +	const struct plat_ipk_fmt *fmt;
>> +
>> +	fmt = vid_dev_find_format(f, -1);
>> +	if (!fmt) {
>> +		f->fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
>> +		fmt = vid_dev_find_format(f, -1);
>> +	}
>> +
>> +	f->fmt.pix.field = V4L2_FIELD_NONE;
>> +	v4l_bound_align_image(&f->fmt.pix.width, 48, MAX_WIDTH, 2,
>> +			      &f->fmt.pix.height, 32, MAX_HEIGHT, 0, 0);
>> +
>> +	f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
>> +	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
>> +	f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
>> +	return 0;
>> +}
>> +
>> +int vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>> +{
>> +	struct video_device_dev *dev = video_drvdata(file);
>> +	int ret;
>> +	struct v4l2_subdev_format fmt;
>> +
>> +	if (vb2_is_busy(&dev->vb_queue))
>> +		return -EBUSY;
>> +
>> +	ret = vidioc_try_fmt_vid_cap(file, dev, f);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dev->fmt = vid_dev_find_format(f, -1);
>> +	pixel_format(dev) = f->fmt.pix.pixelformat;
>> +	width(dev) = f->fmt.pix.width;
>> +	height(dev) = f->fmt.pix.height;
>> +	bytes_per_line(dev) = width(dev) * dev->fmt->depth / 8;
>> +	size_image(dev) = height(dev) * bytes_per_line(dev);
>> +
>> +	fmt.format.colorspace = V4L2_COLORSPACE_SRGB;
>> +	fmt.format.code = dev->fmt->mbus_code;
>> +
>> +	fmt.format.width = width(dev);
>> +	fmt.format.height = height(dev);
>> +
>> +	ret = plat_ipk_pipeline_call(&dev->ve, set_format, &fmt);
>> +
>> +	return 0;
>> +}
>> +
>> +int vidioc_enum_framesizes(struct file *file, void *fh,
>> +		       struct v4l2_frmsizeenum *fsize)
>> +{
>> +	static const struct v4l2_frmsize_stepwise sizes = {
>> +		48, MAX_WIDTH, 4,
>> +		32, MAX_HEIGHT, 1
>> +	};
>> +	int i;
>> +
>> +	if (fsize->index)
>> +		return -EINVAL;
>> +	for (i = 0; i < ARRAY_SIZE(vid_dev_formats); i++)
>> +		if (vid_dev_formats[i].fourcc == fsize->pixel_format)
>> +			break;
>> +	if (i == ARRAY_SIZE(vid_dev_formats))
>> +		return -EINVAL;
>> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
>> +	fsize->stepwise = sizes;
>> +	return 0;
>> +}
>> +
>> +int vidioc_enum_input(struct file *file, void *priv, struct v4l2_input *input)
>> +{
>> +	if (input->index != 0)
>> +		return -EINVAL;
>> +
>> +	input->type = V4L2_INPUT_TYPE_CAMERA;
>> +	input->std = V4L2_STD_ALL;	/* Not sure what should go here */
>> +	strcpy(input->name, "Camera");
>> +	return 0;
>> +}
>> +
>> +int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
>> +{
>> +	*i = 0;
>> +	return 0;
>> +}
>> +
>> +int vidioc_s_input(struct file *file, void *priv, unsigned int i)
>> +{
>> +	if (i != 0)
>> +		return -EINVAL;
>> +	return 0;
>> +}
>> +
>> +int vidioc_g_std(struct file *file, void *fh, v4l2_std_id *norm)
>> +{
>> +	*norm = V4L2_STD_NTSC_M;
>> +	return 0;
>> +}
>> +
>> +int vidioc_s_std(struct file *file, void *fh, v4l2_std_id a)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int
>> +vid_dev_pipeline_validate(struct video_device_dev *vid_dev)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int
>> +vid_dev_streamon(struct file *file, void *priv, enum v4l2_buf_type type)
>> +{
>> +	struct video_device_dev *vid_dev = video_drvdata(file);
>> +	struct media_entity *entity = &vid_dev->ve.vdev.entity;
>> +	int ret;
>> +
>> +	ret = media_entity_pipeline_start(entity, &vid_dev->ve.pipe->mp);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = vid_dev_pipeline_validate(vid_dev);
>> +	if (ret < 0)
>> +		goto err_p_stop;
>> +
>> +	vb2_ioctl_streamon(file, priv, type);
>> +	if (!ret)
>> +		return ret;
>> +
>> +err_p_stop:
>> +	media_entity_pipeline_stop(entity);
>> +	return 0;
>> +}
>> +
>> +static int
>> +vid_dev_streamoff(struct file *file, void *priv, enum v4l2_buf_type type)
>> +{
>> +	struct video_device_dev *vid_dev = video_drvdata(file);
>> +	int ret;
>> +
>> +	ret = vb2_ioctl_streamoff(file, priv, type);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	media_entity_pipeline_stop(&vid_dev->ve.vdev.entity);
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_ioctl_ops vid_dev_ioctl_ops = {
>> +	.vidioc_querycap = vidioc_querycap,
>> +	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
>> +	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
>> +	.vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
>> +	.vidioc_enum_framesizes = vidioc_enum_framesizes,
>> +	.vidioc_enum_input = vidioc_enum_input,
>> +	.vidioc_g_input = vidioc_g_input,
>> +	.vidioc_s_input = vidioc_s_input,
>> +
>> +	.vidioc_reqbufs = vb2_ioctl_reqbufs,
>> +	.vidioc_create_bufs = vb2_ioctl_create_bufs,
>> +	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
>> +	.vidioc_querybuf = vb2_ioctl_querybuf,
>> +	.vidioc_qbuf = vb2_ioctl_qbuf,
>> +	.vidioc_dqbuf = vb2_ioctl_dqbuf,
>> +	.vidioc_streamon = vid_dev_streamon,
>> +	.vidioc_streamoff = vid_dev_streamoff,
>> +};
>> +
>> +/* Capture subdev media entity operations */
>> +static int
>> +vid_dev_link_setup(struct media_entity *entity,
>> +		   const struct media_pad *local,
>> +		   const struct media_pad *remote, u32 flags)
>> +{
>> +	return 0;
>> +}
>> +
>> +static const struct media_entity_operations vid_dev_subdev_media_ops = {
>> +	.link_setup = vid_dev_link_setup,
>> +};
>> +
>> +static int
>> +vid_dev_open(struct file *file)
>> +{
>> +	struct video_device_dev *vid_dev = video_drvdata(file);
>> +	struct media_entity *me = &vid_dev->ve.vdev.entity;
>> +	int ret;
>> +
>> +	mutex_lock(&vid_dev->lock);
>> +
>> +	ret = v4l2_fh_open(file);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	if (!v4l2_fh_is_singular_file(file))
>> +		goto unlock;
>> +
>> +	mutex_lock(&me->graph_obj.mdev->graph_mutex);
>> +
>> +	ret = plat_ipk_pipeline_call(&vid_dev->ve, open, me, true);
>> +	if (ret == 0)
>> +		me->use_count++;
>> +
>> +	mutex_unlock(&me->graph_obj.mdev->graph_mutex);
>> +
>> +	if (!ret)
>> +		goto unlock;
>> +
>> +	v4l2_fh_release(file);
>> +unlock:
>> +	mutex_unlock(&vid_dev->lock);
>> +	return ret;
>> +}
>> +
>> +static int
>> +vid_dev_release(struct file *file)
>> +{
>> +	struct video_device_dev *vid_dev = video_drvdata(file);
>> +	struct media_entity *entity = &vid_dev->ve.vdev.entity;
>> +
>> +	mutex_lock(&vid_dev->lock);
>> +
>> +	if (v4l2_fh_is_singular_file(file)) {
>> +		plat_ipk_pipeline_call(&vid_dev->ve, close);
>> +		mutex_lock(&entity->graph_obj.mdev->graph_mutex);
>> +		entity->use_count--;
>> +		mutex_unlock(&entity->graph_obj.mdev->graph_mutex);
>> +	}
>> +
>> +	_vb2_fop_release(file, NULL);
>> +
>> +	mutex_unlock(&vid_dev->lock);
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_file_operations vid_dev_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = vid_dev_open,
>> +	.release = vid_dev_release,
>> +	.write = vb2_fop_write,
>> +	.read = vb2_fop_read,
>> +	.poll = vb2_fop_poll,
>> +	.unlocked_ioctl = video_ioctl2,
>> +	.mmap = vb2_fop_mmap,
>> +};
>> +
>> +/*
>> + * VideoBuffer2 operations
>> + */
>> +
>> +void fill_buffer(struct video_device_dev *dev, struct rx_buffer *buf,
>> +			int buf_num, unsigned long flags)
>> +{
>> +	int size = 0;
>> +	void *vbuf = NULL;
>> +
>> +	if (&buf->vb == NULL)
>> +		return;
>> +
>> +	size = vb2_plane_size(&buf->vb.vb2_buf, 0);
>> +	vbuf = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
>> +
>> +	if (vbuf) {
>> +		spin_unlock_irqrestore(&dev->slock, flags);
>> +
>> +		memcpy(vbuf, dev->dma_buf[buf_num].cpu_addr, size);
>> +
>> +		spin_lock_irqsave(&dev->slock, flags);
>> +
>> +		buf->vb.field = dev->format.fmt.pix.field;
>> +		buf->vb.sequence++;
>> +		buf->vb.vb2_buf.timestamp = ktime_get_ns();
>> +	}
>> +	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>> +}
>> +
>> +static void buffer_copy_process(void *param)
>> +{
>> +	struct video_device_dev *dev = (struct video_device_dev *) param;
>> +	unsigned long flags;
>> +	struct dmaqueue *dma_q = &dev->vidq;
>> +	struct rx_buffer *buf = NULL;
>> +
>> +	spin_lock_irqsave(&dev->slock, flags);
>> +
>> +	if (!list_empty(&dma_q->active)) {
>> +		buf = list_entry(dma_q->active.next, struct rx_buffer, list);
>> +		list_del(&buf->list);
>> +		fill_buffer(dev, buf, dev->last_idx, flags);
>> +	}
>> +
>> +	spin_unlock_irqrestore(&dev->slock, flags);
>> +}
>> +
>> +static inline struct rx_buffer *to_rx_buffer(struct vb2_v4l2_buffer *vb2)
>> +{
>> +	return container_of(vb2, struct rx_buffer, vb);
>> +}
>> +
>> +int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
>> +			unsigned int *nplanes, unsigned int sizes[],
>> +			struct device *alloc_devs[])
>> +{
>> +	struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> +	unsigned long size = 0;
>> +	int i;
>> +
>> +	size = size_image(dev);
>> +	if (size == 0)
>> +		return -EINVAL;
>> +
>> +	*nbuffers = N_BUFFERS;
>> +
>> +	for (i = 0; i < N_BUFFERS; i++) {
>> +		dev->dma_buf[i].cpu_addr = dma_alloc_coherent(&dev->pdev->dev,
>> +							      size_image(dev),
>> +							      &dev->
>> +							      dma_buf
>> +							      [i].dma_addr,
>> +							      GFP_KERNEL);
>> +	}
>> +
>> +	*nplanes = 1;
>> +	sizes[0] = size;
>> +
>> +	return 0;
>> +}
>> +
>> +int buffer_prepare(struct vb2_buffer *vb)
>> +{
>> +	struct rx_buffer *buf = NULL;
>> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +	int size = 0;
>> +
>> +	if (vb == NULL) {
>> +		pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
>> +		return 0;
>> +	}
>> +
>> +	buf = to_rx_buffer(vbuf);
>> +
>> +	size = vb2_plane_size(&buf->vb.vb2_buf, 0);
>> +	vb2_set_plane_payload(&buf->vb.vb2_buf, 0, size);
>> +
>> +	INIT_LIST_HEAD(&buf->list);
>> +	return 0;
>> +}
>> +
>> +void buffer_queue(struct vb2_buffer *vb)
>> +{
>> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +	struct video_device_dev *dev = NULL;
>> +	struct rx_buffer *buf = NULL;
>> +	struct dmaqueue *vidq = NULL;
>> +	struct dma_async_tx_descriptor *desc;
>> +	u32 flags;
>> +
>> +	if (vb == NULL) {
>> +		pr_warn("%s:vb2_buffer is null\n", FUNC_NAME);
>> +		return;
>> +	}
>> +
>> +	dev = vb2_get_drv_priv(vb->vb2_queue);
>> +	buf = to_rx_buffer(vbuf);
>> +	vidq = &dev->vidq;
>> +
>> +	flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>> +	dev->xt.dir = DMA_DEV_TO_MEM;
>> +	dev->xt.src_sgl = false;
>> +	dev->xt.dst_inc = false;
>> +	dev->xt.dst_sgl = true;
>> +	dev->xt.dst_start = dev->dma_buf[dev->idx].dma_addr;
>> +
>> +	dev->last_idx = dev->idx;
>> +	dev->idx++;
>> +	if (dev->idx >= N_BUFFERS)
>> +		dev->idx = 0;
>> +
>> +	dev->xt.frame_size = 1;
>> +	dev->sgl[0].size = bytes_per_line(dev);
>> +	dev->sgl[0].icg = 0;
>> +	dev->xt.numf = height(dev);
>> +
>> +	desc = dmaengine_prep_interleaved_dma(dev->dma, &dev->xt, flags);
>> +	if (!desc) {
>> +		pr_err("Failed to prepare DMA transfer\n");
>> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>> +		return;
>> +	}
>> +
>> +	desc->callback = buffer_copy_process;
>> +	desc->callback_param = dev;
>> +
>> +	spin_lock(&dev->slock);
>> +	list_add_tail(&buf->list, &vidq->active);
>> +	spin_unlock(&dev->slock);
>> +
>> +	dmaengine_submit(desc);
>> +
>> +	if (vb2_is_streaming(&dev->vb_queue))
>> +		dma_async_issue_pending(dev->dma);
>> +}
>> +
>> +int start_streaming(struct vb2_queue *vq, unsigned int count)
>> +{
>> +	struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> +
>> +	dma_async_issue_pending(dev->dma);
>> +
>> +	return 0;
>> +}
>> +
>> +void stop_streaming(struct vb2_queue *vq)
>> +{
>> +	struct video_device_dev *dev = vb2_get_drv_priv(vq);
>> +	struct dmaqueue *dma_q = &dev->vidq;
>> +
>> +	/* Stop and reset the DMA engine. */
>> +	dmaengine_terminate_all(dev->dma);
>> +
>> +	while (!list_empty(&dma_q->active)) {
>> +		struct rx_buffer *buf;
>> +
>> +		buf = list_entry(dma_q->active.next, struct rx_buffer, list);
>> +		if (buf) {
>> +			list_del(&buf->list);
>> +			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>> +		}
>> +	}
>> +	list_del_init(&dev->vidq.active);
>> +}
>> +
>> +static const struct vb2_ops vb2_video_qops = {
>> +	.queue_setup = queue_setup,
>> +	.buf_prepare = buffer_prepare,
>> +	.buf_queue = buffer_queue,
>> +	.start_streaming = start_streaming,
>> +	.stop_streaming = stop_streaming,
>> +	.wait_prepare = vb2_ops_wait_prepare,
>> +	.wait_finish = vb2_ops_wait_finish,
>> +};
>> +
>> +static int vid_dev_subdev_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> +	return 0;
> 
> I think you could just omit defining the function if it does nothing. The
> same goes for core ops.
> 

This is just here as a placeholder, since this is called in another driver. If
you really think I should remove it I'll have to adapt the other driver to be
prepared for non-existant functions.

>> +}
>> +
>> +static int vid_dev_subdev_registered(struct v4l2_subdev *sd)
>> +{
>> +	struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
>> +	struct vb2_queue *q = &vid_dev->vb_queue;
>> +	struct video_device *vfd = &vid_dev->ve.vdev;
>> +	int ret;
>> +
>> +	memset(vfd, 0, sizeof(*vfd));
>> +
>> +	snprintf(vfd->name, sizeof(vfd->name), VIDEO_DEVICE_NAME);
> 
> How about strlcpy()?
> 
>> +
>> +	vfd->fops = &vid_dev_fops;
>> +	vfd->ioctl_ops = &vid_dev_ioctl_ops;
>> +	vfd->v4l2_dev = sd->v4l2_dev;
>> +	vfd->minor = -1;
>> +	vfd->release = video_device_release_empty;
>> +	vfd->queue = q;
>> +
>> +	INIT_LIST_HEAD(&vid_dev->vidq.active);
>> +	init_waitqueue_head(&vid_dev->vidq.wq);
>> +	memset(q, 0, sizeof(*q));
>> +	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> +	q->io_modes = VB2_MMAP | VB2_USERPTR;
>> +	q->ops = &vb2_video_qops;
>> +	q->mem_ops = &vb2_vmalloc_memops;
>> +	q->buf_struct_size = sizeof(struct rx_buffer);
>> +	q->drv_priv = vid_dev;
>> +	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>> +	q->lock = &vid_dev->lock;
>> +
>> +	ret = vb2_queue_init(q);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	vid_dev->vd_pad.flags = MEDIA_PAD_FL_SINK;
>> +	ret = media_entity_pads_init(&vfd->entity, 1, &vid_dev->vd_pad);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	video_set_drvdata(vfd, vid_dev);
>> +	vid_dev->ve.pipe = v4l2_get_subdev_hostdata(sd);
>> +
>> +	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
>> +	if (ret < 0) {
>> +		media_entity_cleanup(&vfd->entity);
>> +		vid_dev->ve.pipe = NULL;
>> +		return ret;
>> +	}
>> +
>> +	v4l2_info(sd->v4l2_dev, "Registered %s as /dev/%s\n",
>> +		  vfd->name, video_device_node_name(vfd));
>> +	return 0;
>> +}
>> +
>> +static void vid_dev_subdev_unregistered(struct v4l2_subdev *sd)
>> +{
>> +	struct video_device_dev *vid_dev = v4l2_get_subdevdata(sd);
>> +
>> +	if (vid_dev == NULL)
>> +		return;
> 
> Can this happen?
> 

It's the same as before, a better safe than sorry situation. But if you think it
can't happen I'll remove it.

>> +
>> +	mutex_lock(&vid_dev->lock);
>> +
>> +	if (video_is_registered(&vid_dev->ve.vdev)) {
>> +		video_unregister_device(&vid_dev->ve.vdev);
>> +		media_entity_cleanup(&vid_dev->ve.vdev.entity);
>> +		vid_dev->ve.pipe = NULL;
>> +	}
>> +
>> +	mutex_unlock(&vid_dev->lock);
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops vid_dev_subdev_internal_ops = {
>> +	.registered = vid_dev_subdev_registered,
>> +	.unregistered = vid_dev_subdev_unregistered,
>> +};
>> +
>> +static const struct v4l2_subdev_core_ops vid_dev_subdev_core_ops = {
>> +	.s_power = vid_dev_subdev_s_power,
>> +};
>> +
>> +static struct v4l2_subdev_ops vid_dev_subdev_ops = {
>> +	.core = &vid_dev_subdev_core_ops,
>> +};
>> +
>> +static int vid_dev_create_capture_subdev(struct video_device_dev *vid_dev)
>> +{
>> +	struct v4l2_subdev *sd = &vid_dev->subdev;
>> +	int ret;
>> +
>> +	v4l2_subdev_init(sd, &vid_dev_subdev_ops);
>> +	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +	snprintf(sd->name, sizeof(sd->name), "Capture device");
>> +
>> +	vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SINK_CSI].flags =
>> +		MEDIA_PAD_FL_SOURCE;
>> +	vid_dev->subdev_pads[VIDEO_DEV_SD_PAD_SOURCE_DMA].flags =
>> +		MEDIA_PAD_FL_SINK;
>> +	ret =
> 
> No need for a line break.
> 

Right.

>> +	    media_entity_pads_init(&sd->entity, VIDEO_DEV_SD_PADS_NUM,
>> +				   vid_dev->subdev_pads);
>> +	if (ret)
>> +		return ret;
>> +
>> +	sd->internal_ops = &vid_dev_subdev_internal_ops;
>> +	sd->entity.ops = &vid_dev_subdev_media_ops;
>> +	sd->owner = THIS_MODULE;
>> +	v4l2_set_subdevdata(sd, vid_dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static void vid_dev_unregister_subdev(struct video_device_dev *vid_dev)
>> +{
>> +	struct v4l2_subdev *sd = &vid_dev->subdev;
>> +
>> +	v4l2_device_unregister_subdev(sd);
>> +	media_entity_cleanup(&sd->entity);
>> +	v4l2_set_subdevdata(sd, NULL);
>> +}
>> +
>> +static const struct of_device_id vid_dev_of_match[];
>> +
>> +static int vid_dev_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	const struct of_device_id *of_id;
>> +	int ret = 0;
>> +	struct video_device_dev *vid_dev;
>> +
>> +	dev_info(dev, "Installing IPK Video Device module\n");
>> +
>> +	if (!dev->of_node)
>> +		return -ENODEV;
>> +
>> +	vid_dev = devm_kzalloc(dev, sizeof(*vid_dev), GFP_KERNEL);
>> +	if (!vid_dev)
>> +		return -ENOMEM;
>> +
>> +	of_id = of_match_node(vid_dev_of_match, dev->of_node);
>> +	if (WARN_ON(of_id == NULL))
>> +		return -EINVAL;
>> +
>> +	vid_dev->pdev = pdev;
>> +
>> +	spin_lock_init(&vid_dev->slock);
>> +	mutex_init(&vid_dev->lock);
>> +
>> +	dev_info(&pdev->dev, "Requesting DMA\n");
>> +	vid_dev->dma = dma_request_slave_channel(&pdev->dev, "vdma0");
>> +	if (vid_dev->dma == NULL) {
>> +		dev_err(&pdev->dev, "no VDMA channel found\n");
>> +		ret = -ENODEV;
>> +		goto end;
>> +	}
>> +
>> +	ret = vid_dev_create_capture_subdev(vid_dev);
>> +	if (ret)
>> +		goto end;
>> +
>> +	platform_set_drvdata(pdev, vid_dev);
>> +
>> +	dev_info(dev, "Video Device registered successfully\n");
>> +	return 0;
>> +end:
>> +	dev_err(dev, "Video Device not registered!!\n");
>> +	return ret;
>> +}
>> +
>> +static int vid_dev_remove(struct platform_device *pdev)
>> +{
>> +	struct video_device_dev *dev = platform_get_drvdata(pdev);
>> +
>> +	vid_dev_unregister_subdev(dev);
>> +	dev_info(&pdev->dev, "Driver removed\n");
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id vid_dev_of_match[] = {
>> +	{.compatible = "snps,video-device"},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, vid_dev_of_match);
>> +
>> +static struct platform_driver __refdata vid_dev_pdrv = {
>> +	.remove = vid_dev_remove,
>> +	.probe = vid_dev_probe,
>> +	.driver = {
>> +		   .name = VIDEO_DEVICE_NAME,
>> +		   .owner = THIS_MODULE,
>> +		   .of_match_table = vid_dev_of_match,
>> +		   },
>> +};
>> +
>> +module_platform_driver(vid_dev_pdrv);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
>> +MODULE_DESCRIPTION("Driver for configuring DMA and Video Device");
>> diff --git a/drivers/media/platform/dwc/video_device.h b/drivers/media/platform/dwc/video_device.h
>> new file mode 100644
>> index 0000000..e828d4b
>> --- /dev/null
>> +++ b/drivers/media/platform/dwc/video_device.h
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright (C) 2016 Synopsys, Inc. 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 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef VIDEO_DEVICE_H_
>> +#define VIDEO_DEVICE_H_
>> +
>> +#include <linux/module.h>
>> +#include <linux/errno.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/delay.h>
>> +#include <linux/list.h>
>> +#include <linux/wait.h>
>> +#include <linux/string.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/dma/xilinx_dma.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-ioctl.h>
>> +#include <media/v4l2-fh.h>
>> +#include <media/v4l2-common.h>
>> +#include <media/videobuf2-vmalloc.h>
>> +#include <media/media-entity.h>
>> +#include <linux/io.h>
>> +
>> +#include "plat_ipk_video.h"
>> +
>> +#define N_BUFFERS 3
>> +
>> +#define VIDEO_DEVICE_NAME	"video-device"
>> +
>> +#define FUNC_NAME __func__
>> +
>> +struct rx_buffer {
>> +	/** @short Buffer for video frames */
>> +	struct vb2_v4l2_buffer vb;
>> +	struct list_head list;
>> +
>> +	dma_addr_t dma_addr;
>> +	void *cpu_addr;
>> +};
>> +
>> +struct dmaqueue {
>> +	struct list_head active;
>> +	wait_queue_head_t wq;
>> +};
>> +
>> +/**
>> + * @short Structure to embed device driver information
>> + */
>> +struct video_device_dev {
>> +	struct platform_device *pdev;
>> +	struct v4l2_device *v4l2_dev;
>> +	struct v4l2_subdev subdev;
>> +	struct media_pad vd_pad;
>> +	struct media_pad subdev_pads[VIDEO_DEV_SD_PADS_NUM];
>> +	struct mutex lock;
>> +	spinlock_t slock;
>> +	struct plat_ipk_video_entity ve;
>> +	struct v4l2_format format;
>> +	struct v4l2_pix_format pix_format;
>> +	const struct plat_ipk_fmt *fmt;
>> +	unsigned long *alloc_ctx;
>> +
>> +	/* Buffer and DMA */
>> +	struct vb2_queue vb_queue;
>> +	int idx;
>> +	int last_idx;
>> +	struct dmaqueue vidq;
>> +	struct rx_buffer dma_buf[N_BUFFERS];
>> +	struct dma_chan *dma;
>> +	struct dma_interleaved_template xt;
>> +	struct data_chunk sgl[1];
>> +};
>> +
>> +/**
>> + * @short Defines to simplify the code reading
>> + */
>> +
>> +#define pixel_format(dev)	\
>> +	dev->format.fmt.pix.pixelformat
>> +#define bytes_per_line(dev)	\
>> +	dev->format.fmt.pix.bytesperline
>> +#define width(dev)		\
>> +	dev->format.fmt.pix.width
>> +#define height(dev)		\
>> +	dev->format.fmt.pix.height
>> +#define size_image(dev)		\
>> +	dev->format.fmt.pix.sizeimage
> 
> How about referring to the field directly, or define a local variable for a
> pointer to dev->format.fmt.fix in functions it's needed.
> 

Sure, I can do that.

Just to be clear what is the reason why these macros shouldn't be used?

>> +
>> +const struct plat_ipk_fmt *vid_dev_find_format(struct v4l2_format *f,
>> +					       int index);
>> +
>> +#endif				/* VIDEO_DEVICE_H_ */
> 

Thanks once again for you feedback.

BRs,
Ramiro Oliveira

^ permalink raw reply

* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sudeep Holla @ 2016-11-21 17:56 UTC (permalink / raw)
  To: Robin Murphy, Bartosz Golaszewski, Sekhar Nori
  Cc: Sudeep Holla, Mark Rutland, linux-devicetree, Tomi Valkeinen,
	David Airlie, Kevin Hilman, Michael Turquette, Russell King,
	linux-drm, LKML, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc,
	Laurent Pinchart
In-Reply-To: <d0f3bc66-6dfd-dcdc-a15d-a8f9fdda6048-5wv7dgnIgG8@public.gmane.org>

Hi Robin,

On 21/11/16 17:47, Robin Murphy wrote:
> Hi Bartosz, Sekhar,
>
> On 21/11/16 16:48, Bartosz Golaszewski wrote:

[...]

>> Hi Sekhar,
>>
>> thanks for spotting that.
>>
>> I think we should introduce this function right away, rather than
>> having two static functions doing the same thing. If you don't mind,
>> I'll try to find a good spot for it and send a follow-up series fixing
>> the issue.
>
> As it happens, that was already proposed last week, for much the same
> reason:
>
> http://www.mail-archive.com/linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org/msg111395.html
>

Thanks for pointing this out, yet another platform to move to the new
API after v4.10.

Hi Shekar, Bartosz,

For v4.10, please continue with the open coding as proposed in this
thread in order to avoid cross tree dependencies. I will rework on the
above patch once v4.10 merge window closes to include all these
occurrence and replace them.

-- 
Regards,
Sudeep
--
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/5] ARM: memory: da8xx-ddrctl: new driver
From: Robin Murphy @ 2016-11-21 17:47 UTC (permalink / raw)
  To: Bartosz Golaszewski, Sekhar Nori
  Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, David Airlie,
	Kevin Hilman, Michael Turquette, Russell King, linux-drm, LKML,
	Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <CAMpxmJUXJi6PDq0qc-0+r2mLPASLpJUt_njWtXr4Mx4k0Fa82g@mail.gmail.com>

Hi Bartosz, Sekhar,

On 21/11/16 16:48, Bartosz Golaszewski wrote:
> 2016-11-21 17:33 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> +     const struct da8xx_ddrctl_config_knob *knob;
>>> +     const struct da8xx_ddrctl_setting *setting;
>>> +     struct device_node *node;
>>> +     struct resource *res;
>>> +     void __iomem *ddrctl;
>>> +     struct device *dev;
>>> +     u32 reg;
>>> +
>>> +     dev = &pdev->dev;
>>> +     node = dev->of_node;
>>> +
>>> +     setting = da8xx_ddrctl_get_board_settings();
>>> +     if (!setting) {
>>> +             dev_err(dev, "no settings for board '%s'\n",
>>> +                     of_flat_dt_get_machine_name());
>>> +             return -EINVAL;
>>> +     }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name()
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
>>         return NULL;
>>  }
>>
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> +       const char *str;
>> +       int ret;
>> +
>> +       ret = of_property_read_string(of_root, "model", &str);
>> +       if (ret)
>> +               ret = of_property_read_string(of_root, "compatible", &str);
>> +
>> +       return str;
>> +}
>> +
>>  static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>  {
>>         const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>         setting = da8xx_ddrctl_get_board_settings();
>>         if (!setting) {
>>                 dev_err(dev, "no settings for board '%s'\n",
>> -                       of_flat_dt_get_machine_name());
>> +                       da8xx_ddrctl_get_machine_name());
>>                 return -EINVAL;
>>         }
>> ---8<---
>>
>> A similar fix is required for the other driver in this series (patch
>> 2/5). I need some advise on whether I should introduce a common
>> function to get the machine name post kernel boot-up (I cannot see an
>> existing one). If yes, any advise on which file it should go into?
>>
> 
> Hi Sekhar,
> 
> thanks for spotting that.
> 
> I think we should introduce this function right away, rather than
> having two static functions doing the same thing. If you don't mind,
> I'll try to find a good spot for it and send a follow-up series fixing
> the issue.

As it happens, that was already proposed last week, for much the same
reason:

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg111395.html

Robin.

> 
> Best regards,
> Bartosz Golaszewski
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* Re: [PATCH v5 1/2] dt-bindings: display: Add Sharp LQ150X1LG11 panel binding
From: Rob Herring @ 2016-11-21 17:38 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Thierry Reding, David Airlie,
	Mark Rutland, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479740449-20201-2-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On Mon, Nov 21, 2016 at 04:00:48PM +0100, Peter Rosin wrote:
> The Sharp 15" LQ150X1LG11 panel is an XGA TFT LCD panel.
> 
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> ---
>  .../bindings/display/panel/sharp,lq150x1lg11.txt   | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,lq150x1lg11.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: AM571x-IDK Initial Support
From: Rob Herring @ 2016-11-21 17:36 UTC (permalink / raw)
  To: Lokesh Vutla
  Cc: Tony Lindgren, Linux OMAP Mailing List, Tero Kristo, Sekhar Nori,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux ARM Mailing List,
	spatton-l0cyMroinI0, Dave Gerlach, Nishanth Menon
In-Reply-To: <20161121055801.573-1-lokeshvutla-l0cyMroinI0@public.gmane.org>

On Mon, Nov 21, 2016 at 11:28:01AM +0530, Lokesh Vutla wrote:
> From: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
> 
> The AM571x-IDK board is a board based on TI's AM5718 SOC
> which has a single core 1.5GHz A15 processor. This board is a
> development platform for the Industrial market with:
> - 1GB of DDR3L
> - Dual 1Gbps Ethernet
> - HDMI,
> - PRU-ICSS
> - uSD
> - 16GB eMMC
> - CAN
> - RS-485
> - PCIe
> - USB3.0
> - Video Input Port
> - Industrial IO port and expansion connector
> 
> The link to the data sheet and TRM can be found here:
> 
> http://www.ti.com/product/AM5718
> 
> Initial support is only for basic peripherals.
> 
> Signed-off-by: Schuyler Patton <spatton-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> ---
> 
> Logs: http://pastebin.ubuntu.com/23510390/
> 
>  .../devicetree/bindings/arm/omap/omap.txt          |  3 +
>  arch/arm/boot/dts/Makefile                         |  1 +
>  arch/arm/boot/dts/am571x-idk.dts                   | 82 ++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
>  create mode 100644 arch/arm/boot/dts/am571x-idk.dts
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index f53e2ee..647ffd3 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -175,6 +175,9 @@ Boards:
>  - AM5728 IDK
>    compatible = "ti,am5728-idk", "ti,am5728", "ti,dra742", "ti,dra74", "ti,dra7"
>  
> +- AM5718 IDK
> +  compatible = "ti,am5718-idk", "ti,am5728", "ti,dra722", "ti,dra72", "ti,dra7"

I've said this before I think, but 5 compat string is a bit much. Some 
of these genericish ones should be dropped. Doesn't really hurt though.

A couple of nits below, otherwise:

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

> +
>  - DRA742 EVM:  Software Development Board for DRA742
>    compatible = "ti,dra7-evm", "ti,dra742", "ti,dra74", "ti,dra7"
>  
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..c298078 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -588,6 +588,7 @@ dtb-$(CONFIG_SOC_DRA7XX) += \
>  	am57xx-cl-som-am57x.dtb \
>  	am57xx-sbc-am57x.dtb \
>  	am572x-idk.dtb \
> +	am571x-idk.dtb \
>  	dra7-evm.dtb \
>  	dra72-evm.dtb \
>  	dra72-evm-revc.dtb
> diff --git a/arch/arm/boot/dts/am571x-idk.dts b/arch/arm/boot/dts/am571x-idk.dts
> new file mode 100644
> index 0000000..a6a743e
> --- /dev/null
> +++ b/arch/arm/boot/dts/am571x-idk.dts
> @@ -0,0 +1,82 @@
> +/*
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.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.
> + */
> +/dts-v1/;
> +
> +#include "dra72x.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include "am57xx-idk-common.dtsi"
> +
> +/ {
> +	model = "TI AM5718 IDK";
> +	compatible = "ti,am5718-idk", "ti,am5718", "ti,dra722",
> +		     "ti,dra72", "ti,dra7";
> +
> +	memory@0 {

unit address is wrong.

> +		device_type = "memory";
> +		reg = <0x0 0x80000000 0x0 0x40000000>;
> +	};
> +
> +	status-leds {

Just "leds"

> +		compatible = "gpio-leds";
> +		cpu0-led {
> +			label = "status0:red:cpu0";
> +			gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +			linux,default-trigger = "cpu0";
> +		};
> +
> +		usr0-led {
> +			label = "status0:green:usr";
> +			gpios = <&gpio2 26 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +		};
> +
> +		heartbeat-led {
> +			label = "status0:blue:heartbeat";
> +			gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		usr1-led {
> +			label = "status1:red:usr";
> +			gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +		};
> +
> +		usr2-led {
> +			label = "status1:green:usr";
> +			gpios = <&gpio2 21 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +		};
> +
> +		mmc0-led {
> +			label = "status1:blue:mmc0";
> +			gpios = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +			default-state = "off";
> +			linux,default-trigger = "mmc0";
> +		};
> +	};
> +
> +	extcon_usb2: extcon_usb2 {
> +	     compatible = "linux,extcon-usb-gpio";
> +	     id-gpio = <&gpio5 7 GPIO_ACTIVE_HIGH>;
> +	};
> +};
> +
> +&mmc1 {
> +	status = "okay";
> +	vmmc-supply = <&ldo1_reg>;
> +	bus-width = <4>;
> +	cd-gpios = <&gpio6 27 0>; /* gpio 219 */
> +};
> +
> +&omap_dwc3_2 {
> +	extcon = <&extcon_usb2>;
> +};
> -- 
> 2.10.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] spi: sh-msiof: Add support for R-Car M3-W
From: Geert Uytterhoeven @ 2016-11-21 17:24 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland
  Cc: linux-spi, devicetree, linux-renesas-soc, Geert Uytterhoeven

MSIOF in R-Car M3-W (r8a7796) is handled fine by the existing driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Tested with MSIOF2(B) on EXIO connector D of r8a7796/salvator-x, using
spidev, gpio-74x164, and a logic analyzer.

 Documentation/devicetree/bindings/spi/sh-msiof.txt | 1 +
 drivers/spi/spi-sh-msiof.c                         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index aa005c1d10d95756..da6614c6379604bb 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -10,6 +10,7 @@ Required properties:
 			 "renesas,msiof-r8a7792" (R-Car V2H)
 			 "renesas,msiof-r8a7793" (R-Car M2-N)
 			 "renesas,msiof-r8a7794" (R-Car E2)
+			 "renesas,msiof-r8a7796" (R-Car M3-W)
 			 "renesas,msiof-sh73a0" (SH-Mobile AG5)
 - reg                  : A list of offsets and lengths of the register sets for
 			 the device.
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 1de3a772eb7d23a8..0012ad02e5696d35 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -980,6 +980,7 @@ static int sh_msiof_transfer_one(struct spi_master *master,
 	{ .compatible = "renesas,msiof-r8a7792",   .data = &r8a779x_data },
 	{ .compatible = "renesas,msiof-r8a7793",   .data = &r8a779x_data },
 	{ .compatible = "renesas,msiof-r8a7794",   .data = &r8a779x_data },
+	{ .compatible = "renesas,msiof-r8a7796",   .data = &r8a779x_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, sh_msiof_match);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCHv2] arm64: dts: exynos: add the mshc_2 node for supporting T-Flash
From: Krzysztof Kozlowski @ 2016-11-21 17:21 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	krzk-DgEjT+Ai2ygdnm+yROfE0A, cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161121045839.1444-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Mon, Nov 21, 2016 at 01:58:39PM +0900, Jaehoon Chung wrote:
> Add the mshc_2 node for supporting T-flash.
> 
> And it needs to add the "mshc*" aliases. Because dwmmc driver should be
> assigned to "ctrl_id" after parsing to "mshc".
> If there is no aliases for mshc, then it might be set to the wrong
> capabilities.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Changed from 0 to GPIO_ACTIVE_HIGH


Thanks, applied.

Best regards,
Krzysztof
--
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 v4 1/3] Documentation: DT: add dma compatible for sun50i A64 SOC
From: Rob Herring @ 2016-11-21 17:12 UTC (permalink / raw)
  To: Hao Zhang
  Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	vinod.koul-ral2JQCrhuEAvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479638740-20520-2-git-send-email-hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Sun, Nov 20, 2016 at 06:45:38PM +0800, Hao Zhang wrote:
> This add the property of Allwinner sun50i A64 dma.
> 
> Signed-off-by: Hao Zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/sun6i-dma.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-21 17:10 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161121162905.14285-1-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:

> The new Amlogic GXM SoC (S912) is part of the Meson GX family and is nearly
> identical to GXM but with a second Quad-A53 core cluster.
>
> The GXM dtsi includes the GXL dtsi and the p20x dtsi is refactored in a
> common p20x/q20x to support the GXM Q200 and Q201 board that uses the exact
> same board layout since the S905D and S912 are pinout compatible.
>
> The last patch adds support for the Nexbox A1 Set-Top-Box based on the S912.
>
> Changes since RFC :
>  - Refactor the p20x/q20x dtsi into a single common file
>  - Add support for Nexbox A1

Thanks for refactoring.  Looks great.

Applied.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 2/5] drm: sun8i: add HDMI video support to A83T and H3
From: Rob Herring @ 2016-11-21 17:09 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Dave Airlie, Maxime Ripard, devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <cea6a5397664213c6a917028ec2e429d25972490.1479641523.git.moinejf-GANU6spQydw@public.gmane.org>

On Sun, Nov 20, 2016 at 10:56:23AM +0100, Jean-Francois Moine wrote:
> This patch adds a HDMI video driver to the Allwinner's SoCs A83T and H3.
> 
> Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> ---
>  .../devicetree/bindings/display/sunxi/hdmi.txt     |  53 ++
>  drivers/gpu/drm/sun8i/Kconfig                      |   7 +
>  drivers/gpu/drm/sun8i/Makefile                     |   2 +
>  drivers/gpu/drm/sun8i/de2_hdmi.c                   | 394 ++++++++++
>  drivers/gpu/drm/sun8i/de2_hdmi.h                   |  51 ++
>  drivers/gpu/drm/sun8i/de2_hdmi_io.c                | 839 +++++++++++++++++++++
>  6 files changed, 1346 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
>  create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.c
>  create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi.h
>  create mode 100644 drivers/gpu/drm/sun8i/de2_hdmi_io.c
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> new file mode 100644
> index 0000000..85709ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> @@ -0,0 +1,53 @@
> +Allwinner HDMI Transmitter
> +==========================
> +
> +The Allwinner HDMI transmitters are included in the SoCs.
> +They support audio and video.
> +
> +Required properties:
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> + - compatible : should be one of
> +		"allwinner,sun8i-a83t-hdmi"
> +		"allwinner,sun8i-h3-hdmi"
> + - clocks : phandles to the HDMI clocks as described in
> +	Documentation/devicetree/bindings/clock/clock-bindings.txt
> + - clock-names : must be
> +		"gate" : bus gate
> +		"clock" : streaming clock
> +		"ddc-clock" : DDC clock
> + - resets : One or two phandles to the HDMI resets
> + - reset-names : when 2 phandles, must be
> +		"hdmi0" and "hdmi1"
> +
> +Required nodes:
> + - port: Audio and video input port nodes with endpoint definitions
> +	as defined in Documentation/devicetree/bindings/graph.txt.
> +	port@0 is video and port@1 is audio.

This should probably also have an output port to the hdmi-connector 
binding. It is not needed so much if this block handles DDC and HPD 
itself, but if those are a separate I2C controller and GPIO, 
respectively, then you need it for sure. There's also power on the 
connector or other connectors like muxed on Type-C. 


> +
> +Example:
> +
> +	hdmi: hdmi@01ee0000 {
> +		compatible = "allwinner,sun8i-a83t-hdmi";
> +		reg = <0x01ee0000 0x20000>;
> +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> +			 <&ccu CLK_HDMI_DDC>;
> +		clock-names = "gate", "clock", "ddc-clock";
> +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> +		reset-names = "hdmi0", "hdmi1";
> +		...

Please show all properties in example.

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {			/* video */
> +			reg = <0>;
> +			hdmi_lcd1: endpoint {
> +				remote-endpoint = <&lcd1_hdmi>;
> +			};
> +		};
> +		port@1 {			/* audio */
> +			reg = <1>;
> +			hdmi_i2s2: endpoint {
> +				remote-endpoint = <&i2s2_hdmi>;
> +			};
> +		};
> +	};

^ permalink raw reply

* Re: [PATCH 1/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-21 17:06 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161121162905.14285-2-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:

> Following the Amlogic Linux kernel, it seem the only differences
> between the GXL and GXM SoCs are the CPU Clusters.
>
> This commit renames the gxl-s905d-p23x DTSI in a common file for
> S905D p20x and S912 q20x boards.
>
> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
> dts files since the S905D and S912 SoCs shares the same pinout
> and the P23x and Q20x boards are identical.
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/amlogic.txt  |   6 +
>  arch/arm64/boot/dts/amlogic/Makefile               |   2 +
>  .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 190 +++++++++++++++++++++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts    |  19 +++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p201.dts    |   7 +
>  .../boot/dts/amlogic/meson-gxl-s905d-p230.dts      |   3 +-
>  .../boot/dts/amlogic/meson-gxl-s905d-p231.dts      |   3 +-
>  .../boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi     | 188 --------------------
>  .../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts |  77 +++++++++
>  .../arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts |  58 +++++++
>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi         | 114 +++++++++++++
>  11 files changed, 477 insertions(+), 190 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
>  delete mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p23x.dtsi
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm-s912-q201.dts
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxm.dtsi

Applied.

Git tip: for future reference, set 'git config diff.renames true' so and
the diffstat shows a better summary when files are moved/renamed.

e.g, rather than the large ++ and -- above, you'd see:

 ...gxl-s905d-p23x.dtsi => meson-gx-p23x-q20x.dtsi} |   4 +-
 
Which is much more comforting to a maintainer than a bunch of
(potentially unrelated) adds and removes.

Thanks,

Kevin
--
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] ARM: dts: exynos: remove the cd-gpios property for eMMC of odroid-xu3/4
From: Krzysztof Kozlowski @ 2016-11-21 17:06 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	krzk-DgEjT+Ai2ygdnm+yROfE0A, cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
	a.hajda-Sze3O3UU22JBDgjK7y7TUQ,
	jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20161121071032.10183-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Mon, Nov 21, 2016 at 04:10:32PM +0900, Jaehoon Chung wrote:
> Odroid-xu3/4 didn't need to use the cd-gpios for detecting card.
> Because Host controller has the CDETECT register through SDx_CDN line.
> Host controller can know whether card is inserted or not with this
> register.
> 
> When i have checked the Odroid-xu3/4, they are using CDETECT register.
> (Not using exteranl cd-gpio.)

Makes sense. Just one question: the sd0_cd pinctrl setting should stay,
right?

Best regards,
Krzysztof

> Fixes: fb1aeedb61ad ("ARM: dts: add mmc detect gpio for exynos5422-odroidxu3")
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> index 9e63328..05b9afdd 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
> @@ -510,7 +510,6 @@
>  &mmc_0 {
>  	status = "okay";
>  	mmc-pwrseq = <&emmc_pwrseq>;
> -	cd-gpios = <&gpc0 2 GPIO_ACTIVE_LOW>;
>  	card-detect-delay = <200>;
>  	samsung,dw-mshc-ciu-div = <3>;
>  	samsung,dw-mshc-sdr-timing = <0 4>;
> -- 
> 2.10.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 1/5] drm: sun8i: Add a basic DRM driver for Allwinner DE2
From: Rob Herring @ 2016-11-21 16:59 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: devicetree, dri-devel, linux-sunxi, Maxime Ripard,
	linux-arm-kernel
In-Reply-To: <ff2140c86ea3b06406c77d9e8746474b6f8400c4.1479641523.git.moinejf@free.fr>

On Sun, Nov 20, 2016 at 10:53:25AM +0100, Jean-Francois Moine wrote:
> Allwinner's recent SoCs, as A64, A83T and H3, contain a new display
> engine, DE2.
> This patch adds a DRM video driver for this device.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  .../bindings/display/sunxi/sun8i-de2.txt           |  83 +++

It's preferred to split bindings to a separate patch.

>  drivers/gpu/drm/Kconfig                            |   2 +
>  drivers/gpu/drm/Makefile                           |   1 +
>  drivers/gpu/drm/sun8i/Kconfig                      |  19 +
>  drivers/gpu/drm/sun8i/Makefile                     |   7 +
>  drivers/gpu/drm/sun8i/de2_crtc.c                   | 440 +++++++++++++
>  drivers/gpu/drm/sun8i/de2_crtc.h                   |  50 ++
>  drivers/gpu/drm/sun8i/de2_drm.h                    |  48 ++
>  drivers/gpu/drm/sun8i/de2_drv.c                    | 379 +++++++++++
>  drivers/gpu/drm/sun8i/de2_plane.c                  | 712 +++++++++++++++++++++
>  10 files changed, 1741 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
>  create mode 100644 drivers/gpu/drm/sun8i/Kconfig
>  create mode 100644 drivers/gpu/drm/sun8i/Makefile
>  create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.c
>  create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.h
>  create mode 100644 drivers/gpu/drm/sun8i/de2_drm.h
>  create mode 100644 drivers/gpu/drm/sun8i/de2_drv.c
>  create mode 100644 drivers/gpu/drm/sun8i/de2_plane.c
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> new file mode 100644
> index 0000000..b9edd4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> @@ -0,0 +1,83 @@
> +Allwinner sun8i Display Engine 2 subsystem
> +==========================================
> +
> +The Allwinner DE2 subsystem contains a display controller (DE2),
> +one or two LCD controllers (TCON) and their external interfaces.
> +
> +Display controller
> +==================
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> +		"allwinner,sun8i-a83t-display-engine"
> +		"allwinner,sun8i-h3-display-engine"
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> +		clock-names property.
> +
> +- clock-names: must contain
> +		"gate": DE bus gate
> +		"clock": DE clock
> +
> +- resets: phandle to the reset of the device
> +
> +- ports: phandle's to the LCD ports

This should use OF graph to describe the connection from the DE to the 
LCD controllers like the sun4i binding does.

No registers for the DE?

> +
> +LCD controller
> +==============
> +
> +Required properties:
> +
> +- compatible: should be
> +		"allwinner,sun8i-a83t-tcon"
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> +		clock-names property.
> +
> +- clock-names: must contain
> +		"gate": TCON bus gate
> +		"clock": TCON pixel clock
> +
> +- resets: phandle to the reset of the device
> +
> +- port: port node with endpoint definitions as defined in
> +	Documentation/devicetree/bindings/media/video-interfaces.txt

Need to specify how many ports and endpoints.

> +
> +Example:
> +
> +	de: de-controller@01000000 {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		...

What are you not showing?

> +		clocks = <&&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> +		clock-names = "gate", "clock";
> +		resets = <&ccu RST_BUS_DE>;
> +		ports = <&lcd0_p>;
> +	};
> +
> +	lcd0: lcd-controller@01c0c000 {
> +		compatible = "allwinner,sun8i-a83t-tcon";
> +		...

ditto.

> +		clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> +		clock-names = "gate", "clock";
> +		resets = <&ccu RST_BUS_TCON0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		lcd0_p: port {
> +			lcd0_ep: endpoint {
> +				remote-endpoint = <&hdmi_ep>;
> +			};
> +		};
> +	};
> +
> +	hdmi: hdmi@01ee0000 {
> +		...
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port {
> +			hdmi_ep: endpoint {
> +				remote-endpoint = <&lcd0_ep>;
> +			};
> +		};
> +	};
> +
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 03/13] devicetree/bindings: display: Add bindings for two Mitsubishi panels
From: Rob Herring @ 2016-11-21 16:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: dri-devel, linux-renesas-soc, Tomi Valkeinen, devicetree
In-Reply-To: <1479526093-7014-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

On Sat, Nov 19, 2016 at 05:28:03AM +0200, Laurent Pinchart wrote:
> The AA104XD12 and AA121TD01 are LVDS display panels. Their bindings are
> modelled on the the LVS panel bindings.

s/LVS/LVDS/

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../display/panel/mitsubishi,aa104xd12.txt         | 47 ++++++++++++++++++++++
>  .../display/panel/mitsubishi,aa121td01.txt         | 47 ++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa104xd12.txt
>  create mode 100644 Documentation/devicetree/bindings/display/panel/mitsubishi,aa121td01.txt

With that,

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

^ permalink raw reply

* [PATCH] ARM: dts: sunxi: Enable UEXT related nodes for Olimex A20 SOM EVB
From: Emmanuel Vadot @ 2016-11-21 16:49 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, maxime.ripard, wens
  Cc: devicetree, Emmanuel Vadot, linux-kernel, linux-arm-kernel

UEXT are Universal EXTension connector from Olimex. They embed i2c, spi
and uart pins along power in one connector and are found on most,
if not all, Olimex boards.
The Olimex A20 SOM EVB have two UEXT connector so enable the nodes found on
those two connectors.

Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com>
---
 arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts | 36 ++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
index 23aacce..e879c119 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
@@ -116,6 +116,18 @@
 	};
 };
 
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+	status = "okay";
+};
+
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins_a>;
+	status = "okay";
+};
+
 &lradc {
 	vref-supply = <&reg_vcc3v0>;
 	status = "okay";
@@ -284,12 +296,36 @@
 	status = "okay";
 };
 
+&spi1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi1_pins_a>,
+		<&spi1_cs0_pins_a>;
+};
+
+&spi2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi2_pins_a>,
+		<&spi2_cs0_pins_a>;
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
 	status = "okay";
 };
 
+&uart6 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart6_pins_a>;
+	status = "okay";
+};
+
+&uart7 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart7_pins_a>;
+	status = "okay";
+};
+
 &usb_otg {
 	dr_mode = "otg";
 	status = "okay";
-- 
2.9.2

^ permalink raw reply related


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