linux-kernel - Re: [PATCH 2/3] tegra: pwm-backlight: add tegra pwm-bl driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50FCFDD9.6000203@gmail.com>
Date:	Mon, 21 Jan 2013 16:35:37 +0800
From:	Mark Zhang <nvmarkzhang@...il.com>
To:	Alex Courbot <acourbot@...dia.com>
CC:	Thierry Reding <thierry.reding@...onic-design.de>,
	Stephen Warren <swarren@...dotorg.org>,
	"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	Mark Zhang <markz@...dia.com>,
	"gnurou@...il.com" <gnurou@...il.com>
Subject: Re: [PATCH 2/3] tegra: pwm-backlight: add tegra pwm-bl driver

On 01/21/2013 04:24 PM, Alex Courbot wrote:
> On Monday 21 January 2013 15:35:58 Mark Zhang wrote:
>>> +	backlight {
>>> +		compatible = "pwm-backlight-ventana";
>>> +		brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 
> 208
>>> 224 240 255>; +		default-brightness-level = <12>;
>>> +
>>> +		pwms = <&pwm 2 5000000>;
>>
>> After read the codes of tegra pwm driver & pwm framework, I got to know
>> the meaning of this property. So I think we need to add a doc(e.g:
>> Documentation/devicetree/bindings/video/backlight/nvidia,tegra20-bl.txt)
>> to explain this, "Documentation/devicetree/bindings/pwm/pwm.txt" doesn't
>> explain this, because this may be different between different pwm drivers.
> 
> The bindings are in Documentation/devicetree/bindings/video/backlight/pwm-
> backlight.txt . But you are right that the power supplies and GPIO will 
> require a description of their own - I omitted it for this version because I 
> am not sure what the driver should be called.
> 

The description of this property in pwm-backlight.txt is:

"pwms: OF device-tree PWM specification (see PWM binding[0])
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt"

So you can't get any useful infos from that. That's why I propose to add
a tegra specific doc in
"Documentation/devicetree/bindings/video/backlight" directory.

> The panel used on Ventana is a Chunghwa CLAA101WA01A, maybe that's the name we 
> should use for the compatible string instead (and rename the driver 
> accordingly).
> 
>> So according to the filename, I think we can put all tegra boards codes
>> here, right? Just like what you do for Ventana, if I wanna add support
>> for cardhu, I can define similar functions -- let's say "init_cardhu",
>> "exit_cardhu", "notify_cardhu" and "notify_after_cardhu", right?
> 
> That was my initial intention, yes.
> 
>> But I think if we do in this way, the file will become very long soon.
>> And there are a lot of redundant codes in it. So do you have any
>> suggestions?
> 
> If we decide to make a "Tegra" driver, then I don't think the size of the file 
> is a big issues, as long as one can easily navigate into it. It will make 
> sense to do this since Tegra kernels should include support for all the 
> boards.
> 
> If we go and name the drivers after their actual panel names, we should 
> definitely put them into separate files. The Tegra configuration could then 
> include them all by default to make sure all boards are supported.

I don't think use panel name instead of board name is a good idea.
Developers may not be familiar with panel names. So if we use panel
name, we have to search and read a lot of manual to find out what the
panel is.

I'd rather putting all stuffs in pwm_bl_tegra.c than separating them.

Mark
> 
> Alex.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/