linux-kernel - Re: [PATCH] serial/efm32: add new 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: <20120109103458.GD14252@pengutronix.de>
Date:	Mon, 9 Jan 2012 11:34:58 +0100
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-kernel@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
	devicetree-discuss@...ts.ozlabs.org,
	Greg Kroah-Hartman <gregkh@...e.de>, kernel@...gutronix.de,
	linux-serial@...r.kernel.org
Subject: Re: [PATCH] serial/efm32: add new driver

Hello,

On Fri, Dec 23, 2011 at 09:44:28PM +0100, Uwe Kleine-König wrote:
> On Fri, Dec 23, 2011 at 10:35:22AM +0000, Arnd Bergmann wrote:
> > On Thursday 22 December 2011, Uwe Kleine-König wrote:
> > 
> > > @@ -0,0 +1,14 @@
> > > +* Energymicro efm32 UART
> > > +
> > > +Required properties:
> > > +- compatible : Should be "efm32,usart"
> > > +- reg : Address and length of the register set
> > > +- interrupts : Should contain uart interrupt
> > > +
> > > +Example:
> > > +
> > > +uart@...000c400 {
> > > +	compatible = "efm32,usart";
> > > +	reg = <0x4000c400 0x400>;
> > > +	interrupts = <15>;
> > > +};
> > 
> > Do you know if the usart was actually designed by energymicro or licensed
> > from another party? If it is a licensed part, it would be better to
> > list the "compatible" value under the company name that made it.
> I don't know so I passed the question to them.
I got an answer here over the holidays and Energy Micro designed the
part.
 
> > I would suggest that you also support the "clock-frequency" and/or
> > "current-speed" properties that are defined for serial ports, see the
> > of_serial driver.
> I will have a look to find out what they mean and update the patch
> accordingly.
I took that look and I don't understand what they are good for in my
case.
clock-frequency is used to initialize port->uartclk (and helps setting
up port->custom_divisor if current-speed is given). The driver I posted
uses the clk API to determine the clock frequency. So shouldn't the frequency
better be specified in the clk part of the dt? (I don't know yet how a
dt binding for clks should/can look like, so this is a bit speculative,
but I'd expect to have nothing more clk related in a device
specification but a reference to a clk definition.)

Independant of my driver I wonder if defining current-speed should also
result in

	port->flags |= UPF_SPD_CUST;

(in of_platform_serial_setup()).

Having said that and taking into account that my driver doesn't use
port->custom_divisor, do you keep suggesting that I should use
"clock-frequency" and/or "current-speed"?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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/