All of lore.kernel.org
 help / color / mirror / Atom feed
From: alexander.stein@systec-electronic.com (Alexander Stein)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver
Date: Tue, 29 Mar 2016 09:26:53 +0200	[thread overview]
Message-ID: <3752084.bcROvR6fvG@ws-stein> (raw)
In-Reply-To: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch>

On Tuesday 29 March 2016 00:11:13, Stefan Agner wrote:
> >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> 
> >> @@ -14,6 +14,7 @@ Required properties:
> >>  Optional properties:
> >>  - clocks:		Second handle for pixel clock.
> >>  - clock-names:		Second name "pix" for pixel clock.
> >> 
> >> +- fsl,tcon:		The phandle to the timing controller node.
> > 
> > Maybe a comment this is (currently) only for Vybrid, but not LS1021A.
> 
> IMHO, such references to SoCs in a peripheral binding is somewhat
> misplaced. If we would start doing this, we would end up in lots of SoC
> references, and nobody knows whether they are actually accurate and up
> to date... And I think we should add tcon to LS1021a, more on that
> below...

Well, I don't mind after all.

> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> >> [...]
> >> +struct fsl_tcon *fsl_tcon_init(struct device *dev)
> >> +{
> >> +	struct fsl_tcon *tcon;
> >> +	struct device_node *np;
> >> +	int ret;
> >> +
> >> +	tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
> >> +	if (!tcon)
> >> +		return NULL;
> >> +
> >> +	np = of_parse_phandle(dev->of_node, "fsl,tcon", 0);
> >> +	if (!np) {
> >> +		dev_warn(dev, "Couldn't find the tcon node\n");
> >> +		return NULL;
> >> +	}
> > 
> > Maybe check for device tree node before allocating struct fsl_tcon? Also
> > this should not be a warning, as on LS1021A this is to be expected.
> 
> Agreed, definitely the smarter sequence.
> 
> Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches
> it was part of the driver, see:
> https://lkml.org/lkml/2015/7/13/242
> 
> Not sure how that driver can work without TCON support on LS1021a, maybe
> the bootloader sets the TCON to bypass?
> 
> Since I do not have a LS1021a, I hesitated to just add it to the LS1021s
> dt's, but it should be fairly straight forward.

Interesting, but in Patch 3 (https://lists.freedesktop.org/archives/dri-devel/2015-July/086381.html) no TCON node is added and using is still 
optional. Yet, I don't find any TCON hardware in the LS1021A reference manual, 
e.g. in the memory map. I hard guess is that it doesn't require/use one. 
Without memory address it would be pretty hard to add one.

Alexander

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Stein <alexander.stein@systec-electronic.com>
To: Stefan Agner <stefan@agner.ch>
Cc: meng.yi@nxp.com, pawel.moll@arm.com, alison.wang@freescale.com,
	daniel.vetter@ffwll.ch, mturquette@baylibre.com,
	ijc+devicetree@hellion.org.uk, sboyd@codeaurora.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	devicetree@vger.kernel.org, robh+dt@kernel.org,
	kernel@pengutronix.de, galak@codeaurora.org,
	mark.rutland@arm.com, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver
Date: Tue, 29 Mar 2016 09:26:53 +0200	[thread overview]
Message-ID: <3752084.bcROvR6fvG@ws-stein> (raw)
In-Reply-To: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch>

On Tuesday 29 March 2016 00:11:13, Stefan Agner wrote:
> >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> 
> >> @@ -14,6 +14,7 @@ Required properties:
> >>  Optional properties:
> >>  - clocks:		Second handle for pixel clock.
> >>  - clock-names:		Second name "pix" for pixel clock.
> >> 
> >> +- fsl,tcon:		The phandle to the timing controller node.
> > 
> > Maybe a comment this is (currently) only for Vybrid, but not LS1021A.
> 
> IMHO, such references to SoCs in a peripheral binding is somewhat
> misplaced. If we would start doing this, we would end up in lots of SoC
> references, and nobody knows whether they are actually accurate and up
> to date... And I think we should add tcon to LS1021a, more on that
> below...

Well, I don't mind after all.

> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> >> [...]
> >> +struct fsl_tcon *fsl_tcon_init(struct device *dev)
> >> +{
> >> +	struct fsl_tcon *tcon;
> >> +	struct device_node *np;
> >> +	int ret;
> >> +
> >> +	tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
> >> +	if (!tcon)
> >> +		return NULL;
> >> +
> >> +	np = of_parse_phandle(dev->of_node, "fsl,tcon", 0);
> >> +	if (!np) {
> >> +		dev_warn(dev, "Couldn't find the tcon node\n");
> >> +		return NULL;
> >> +	}
> > 
> > Maybe check for device tree node before allocating struct fsl_tcon? Also
> > this should not be a warning, as on LS1021A this is to be expected.
> 
> Agreed, definitely the smarter sequence.
> 
> Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches
> it was part of the driver, see:
> https://lkml.org/lkml/2015/7/13/242
> 
> Not sure how that driver can work without TCON support on LS1021a, maybe
> the bootloader sets the TCON to bypass?
> 
> Since I do not have a LS1021a, I hesitated to just add it to the LS1021s
> dt's, but it should be fairly straight forward.

Interesting, but in Patch 3 (https://lists.freedesktop.org/archives/dri-devel/2015-July/086381.html) no TCON node is added and using is still 
optional. Yet, I don't find any TCON hardware in the LS1021A reference manual, 
e.g. in the memory map. I hard guess is that it doesn't require/use one. 
Without memory address it would be pretty hard to add one.

Alexander

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Stein <alexander.stein@systec-electronic.com>
To: Stefan Agner <stefan@agner.ch>
Cc: dri-devel@lists.freedesktop.org, shawnguo@kernel.org,
	kernel@pengutronix.de, airlied@linux.ie, daniel.vetter@ffwll.ch,
	jianwei.wang.chn@gmail.com, alison.wang@freescale.com,
	meng.yi@nxp.com, mturquette@baylibre.com, sboyd@codeaurora.org,
	mark.rutland@arm.com, robh+dt@kernel.org, pawel.moll@arm.com,
	ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 6/8] drm/fsl-dcu: add TCON driver
Date: Tue, 29 Mar 2016 09:26:53 +0200	[thread overview]
Message-ID: <3752084.bcROvR6fvG@ws-stein> (raw)
In-Reply-To: <57c86de8eca0aa16a5cdfc488977ca4e@agner.ch>

On Tuesday 29 March 2016 00:11:13, Stefan Agner wrote:
> >> --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> >> 
> >> @@ -14,6 +14,7 @@ Required properties:
> >>  Optional properties:
> >>  - clocks:		Second handle for pixel clock.
> >>  - clock-names:		Second name "pix" for pixel clock.
> >> 
> >> +- fsl,tcon:		The phandle to the timing controller node.
> > 
> > Maybe a comment this is (currently) only for Vybrid, but not LS1021A.
> 
> IMHO, such references to SoCs in a peripheral binding is somewhat
> misplaced. If we would start doing this, we would end up in lots of SoC
> references, and nobody knows whether they are actually accurate and up
> to date... And I think we should add tcon to LS1021a, more on that
> below...

Well, I don't mind after all.

> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> >> [...]
> >> +struct fsl_tcon *fsl_tcon_init(struct device *dev)
> >> +{
> >> +	struct fsl_tcon *tcon;
> >> +	struct device_node *np;
> >> +	int ret;
> >> +
> >> +	tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
> >> +	if (!tcon)
> >> +		return NULL;
> >> +
> >> +	np = of_parse_phandle(dev->of_node, "fsl,tcon", 0);
> >> +	if (!np) {
> >> +		dev_warn(dev, "Couldn't find the tcon node\n");
> >> +		return NULL;
> >> +	}
> > 
> > Maybe check for device tree node before allocating struct fsl_tcon? Also
> > this should not be a warning, as on LS1021A this is to be expected.
> 
> Agreed, definitely the smarter sequence.
> 
> Afact LS1021a has also a TCON. At least in Jianwei Wangs initial patches
> it was part of the driver, see:
> https://lkml.org/lkml/2015/7/13/242
> 
> Not sure how that driver can work without TCON support on LS1021a, maybe
> the bootloader sets the TCON to bypass?
> 
> Since I do not have a LS1021a, I hesitated to just add it to the LS1021s
> dt's, but it should be fairly straight forward.

Interesting, but in Patch 3 (https://lists.freedesktop.org/archives/dri-devel/2015-July/086381.html) no TCON node is added and using is still 
optional. Yet, I don't find any TCON hardware in the LS1021A reference manual, 
e.g. in the memory map. I hard guess is that it doesn't require/use one. 
Without memory address it would be pretty hard to add one.

Alexander

  reply	other threads:[~2016-03-29  7:26 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29  1:59 [PATCH v2 0/8] add TCON and Vybrid support Stefan Agner
2016-03-29  1:59 ` Stefan Agner
2016-03-29  1:59 ` Stefan Agner
2016-03-29  1:59 ` [PATCH v2 1/8] ARM: imx: clk-vf610: fix DCU clock tree Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  1:59 ` [PATCH v2 2/8] ARM: imx: clk-vf610: add TCON ipg clock Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  1:59 ` [PATCH v2 3/8] drm/fsl-dcu: disable clock on initialization failure and remove Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  1:59 ` [PATCH v2 4/8] drm/fsl-dcu: add extra clock for pixel clock Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-31 14:42   ` Rob Herring
2016-03-31 14:42     ` Rob Herring
2016-03-31 14:42     ` Rob Herring
2016-03-29  1:59 ` [PATCH v2 5/8] drm/fsl-dcu: use common clock framework for pixel clock divider Stefan Agner
2016-03-29  1:59   ` Stefan Agner
2016-03-29  2:00 ` [PATCH v2 6/8] drm/fsl-dcu: add TCON driver Stefan Agner
2016-03-29  2:00   ` Stefan Agner
2016-03-29  6:45   ` Alexander Stein
2016-03-29  6:45     ` Alexander Stein
2016-03-29  6:45     ` Alexander Stein
2016-03-29  7:11     ` Stefan Agner
2016-03-29  7:11       ` Stefan Agner
2016-03-29  7:11       ` Stefan Agner
2016-03-29  7:26       ` Alexander Stein [this message]
2016-03-29  7:26         ` Alexander Stein
2016-03-29  7:26         ` Alexander Stein
2016-03-29  7:39         ` Stefan Agner
2016-03-29  7:39           ` Stefan Agner
2016-03-29  7:39           ` Stefan Agner
2016-03-31 14:35   ` Rob Herring
2016-03-31 14:35     ` Rob Herring
2016-03-31 14:35     ` Rob Herring
2016-03-29  2:00 ` [PATCH v2 7/8] ARM: dts: vf610: add display nodes Stefan Agner
2016-03-29  2:00   ` Stefan Agner
2016-03-29  2:00 ` [PATCH v2 8/8] ARM: dts: vf610-colibri: enable display controller Stefan Agner
2016-03-29  2:00   ` Stefan Agner
2016-03-29  2:00   ` Stefan Agner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3752084.bcROvR6fvG@ws-stein \
    --to=alexander.stein@systec-electronic.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.