linux-kernel - Re: [PATCH 5/7] PPS: serial clients support.

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: <20080502073335.GK3002@enneenne.com>
Date:	Fri, 2 May 2008 09:33:35 +0200
From:	Rodolfo Giometti <giometti@...eenne.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Lennart Sorensen <lsorense@...lub.uwaterloo.ca>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Woodhouse <dwmw2@...radead.org>,
	Dave Jones <davej@...hat.com>, Sam Ravnborg <sam@...nborg.org>,
	Greg KH <greg@...ah.com>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	Kay Sievers <kay.sievers@...y.org>
Subject: Re: [PATCH 5/7] PPS: serial clients support.

On Wed, Apr 30, 2008 at 05:28:55PM +0100, Alan Cox wrote:
> > if I add a dedicated line discipline to register/unregister the PPS
> > source and I leave the pps_event management into
> > uart_handle_dcd_change() function, it can be acceptable?
> 
> Keep the two things apart.
> 
> uart_handle_dcd_change looks a good basis for the UART layer support for
> drivers serial
> an LDISC looks right for the top layer
> 
> We just need the bits in the middle right. I've added the serial layer
> pass through for the set_ldisc() interface so that bit is done. Probably
> the main thing we need is to add  tty->ldisc.dcd_change() for reporting
> DCD change back to the line discipline. We could queue it as a TTY_ event
> but I assume you need it immediately not queued ?

Yes, you are right. PPS events should be registered as soon as they
arrive, that's why I'd like to put events registration (or at least
IRQ timestamps) into do_IRQ() as follow:

   fastcall unsigned int do_IRQ(struct pt_regs *regs)
   {
           struct pt_regs *old_regs;
           /* high bit used in ret_from_ code */
           int irq = ~regs->orig_eax;
           struct irq_desc *desc = irq_desc + irq;
   #ifdef CONFIG_4KSTACKS
           union irq_ctx *curctx, *irqctx;
           u32 *isp;
   #endif

           if (unlikely((unsigned)irq >= NR_IRQS)) {
                   printk(KERN_EMERG "%s: cannot handle IRQ %d\n",
                                           __FUNCTION__, irq);
                   BUG();
           }

           pps_irq_event_register(irq);

           old_regs = set_irq_regs(regs);
           irq_enter();

but this solution arises two drawbacks:

1) the PPS echo functions should be executed by
pps_irq_event_register() into do_IRQ(). Brrrr... :)

2) the shared IRQ line may cause "false" PPS events.

The latter drawback can be solved simply not using the shared IRQs for
PPS sources but the former is not an easy task... so I suppose we
should delay the echo function call at "safer" time, maybe using the
function tty->ldisc.dcd_change().

> > The uart_handle_dcd_change() is generic and I need the DCD status to
> > correctly manage the pps_event. The USB layer is not useful for PPS
> > stuff
> 
> Not every character driver uses drivers/serial or USB. That is fine. What
> I care about is that you *could* add PPS support to another serial driver
> cleanly, not that it is done immediately. What matters is the interface,
> the rest will follow.

I see. So what do you suggest to do? Should I add an ldisc to
register/unregister serial PPS sources and adding function
tty->ldisc.dcd_change() to manage the events?

Thanks for your suggestions,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@...eenne.com
Linux Device Driver                             giometti@...ux.it
Embedded Systems                     phone:	+39 349 2432127
UNIX programming                     skype:     rodolfo.giometti
--
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/