All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ralf Baechle <ralf@linux-mips.org>
To: Gerhard Pircher <gerhard_pircher@gmx.net>
Cc: linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org,
	linux-alpha@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [patch 00/14] Sort out i8253 and PC speaker locking and headers
Date: Fri, 3 Jun 2011 19:00:38 +0100	[thread overview]
Message-ID: <20110603180038.GA13239@linux-mips.org> (raw)
In-Reply-To: <20110602191119.302850@gmx.net>

On Thu, Jun 02, 2011 at 09:11:19PM +0200, Gerhard Pircher wrote:

> > #if defined(CONFIG_MIPS) || defined(CONFIG_X86)
> > /* Use the global PIT lock ! */
> > #include <asm/i8253.h>
> > #else
> > #include <asm/8253pit.h>
> > static DEFINE_RAW_SPINLOCK(i8253_lock);
> > #endif
> > 
> > sound/drivers/pcsp/pcsp.h:
> > 
> > #if defined(CONFIG_MIPS) || defined(CONFIG_X86)
> > /* Use the global PIT lock ! */
> > #include <asm/i8253.h>
> > #else
> > #include <asm/8253pit.h>
> > static DEFINE_RAW_SPINLOCK(i8253_lock);
> > 
> > $ git grep -F pcsp.h sound/drivers/pcsp
> > sound/drivers/pcsp/pcsp.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_input.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_lib.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_mixer.c:#include "pcsp.h"
> > $ git grep -w i8253_lock sound/drivers/pcsp/
> > sound/drivers/pcsp/pcsp.h:static DEFINE_RAW_SPINLOCK(i8253_lock);
> > sound/drivers/pcsp/pcsp_input.c:        raw_spin_lock_irqsave(&i8253_lock,
> > flags
> > sound/drivers/pcsp/pcsp_input.c:       
> > raw_spin_unlock_irqrestore(&i8253_lock, 
> > sound/drivers/pcsp/pcsp_lib.c:          raw_spin_lock_irqsave(&i8253_lock,
> > flags
> > sound/drivers/pcsp/pcsp_lib.c:         
> > raw_spin_unlock_irqrestore(&i8253_lock, 
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_lock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_unlock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_lock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_unlock(&i8253_lock);
> > 
> > Locks are great, everybody should have their own lock!
> > 
> > $ find . -name 8253pit.h
> > ./arch/powerpc/include/asm/8253pit.h
> > ./arch/alpha/include/asm/8253pit.h
> > $ cat arch/*/include/asm/8253pit.h
> > /*
> >  * 8253/8254 Programmable Interval Timer
> >  */
> > /*
> >  * 8253/8254 Programmable Interval Timer
> >  */
> > $
> > 
> > Eh...
> > 
> > $ git grep -w PCSPKR_PLATFORM 
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/powerpc/platforms/amigaone/Kconfig:        select PCSPKR_PLATFORM
> > drivers/input/misc/Kconfig:     depends on PCSPKR_PLATFORM
> > init/Kconfig:config PCSPKR_PLATFORM
> > sound/drivers/Kconfig:  depends on PCSPKR_PLATFORM && X86 &&
> > HIGH_RES_TIMERS
> > 
> > So the status is:
> > 
> >  Alpha:	There is no PCSPKR_PLATFORM so while a platform device is
> > 		being installed no drivers will be built.  I don't know
> > 		which Alpha platforms or even if all of Alpha should be
> > 		doing a PCSPKR_PLATFORM so I haven't even tried to sort
> > 		this.
> >  ARM:		No PC speaker supported, yeah :)
> >  PowerPC:	Should compile but the locking is wrong but only the 
> >    		AmigaOne platforms should be affected.
> The Kconfig dependencies cleanup patch for CHRP, PSERIES, etc. should
> also apply to the AmigaOne. Can you resend it with a fix for the
> AmigaOne, or should I send a patch?

I can sort that; it's easy enough.

> I'll check next week, if the PC speaker is still working on my AmigaOne.

I can't imagine that it's going to break - the code is sorta simple ;-)

One obscurity I noticed is this bit in the amigaone.dts:

        timer@40 {
                // Also adds pcspkr to platform devices.
                compatible = "pnpPNP,100";
                reg = <1 0x00000040 0x00000020>;
        };

Shouldn't that rather be something like the following?

        pcspeaker@61 {
                device_type = "sound";
                compatible = "pnpPNP,800";
                reg = <1 0x61 1>;
        };

pnpPNP,100 is the i8253 timer as I understand and pnpPNP,800 the PC speaker.
If you interpret pnpPNP,100 to imply the presence of a PC speaker you can't
express a system that has a i8253 but no PCspeaker in a DT so maybe
amigaone.dts and arch/powerpc/kernel/setup-common.c should be changed to
use pnpPNP,800 instead?

  Ralf

WARNING: multiple messages have this Message-ID (diff)
From: Ralf Baechle <ralf@linux-mips.org>
To: Gerhard Pircher <gerhard_pircher@gmx.net>
Cc: linux-arch@vger.kernel.org, linux-mips@linux-mips.org,
	linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [patch 00/14] Sort out i8253 and PC speaker locking and headers
Date: Fri, 3 Jun 2011 19:00:38 +0100	[thread overview]
Message-ID: <20110603180038.GA13239@linux-mips.org> (raw)
In-Reply-To: <20110602191119.302850@gmx.net>

On Thu, Jun 02, 2011 at 09:11:19PM +0200, Gerhard Pircher wrote:

> > #if defined(CONFIG_MIPS) || defined(CONFIG_X86)
> > /* Use the global PIT lock ! */
> > #include <asm/i8253.h>
> > #else
> > #include <asm/8253pit.h>
> > static DEFINE_RAW_SPINLOCK(i8253_lock);
> > #endif
> > 
> > sound/drivers/pcsp/pcsp.h:
> > 
> > #if defined(CONFIG_MIPS) || defined(CONFIG_X86)
> > /* Use the global PIT lock ! */
> > #include <asm/i8253.h>
> > #else
> > #include <asm/8253pit.h>
> > static DEFINE_RAW_SPINLOCK(i8253_lock);
> > 
> > $ git grep -F pcsp.h sound/drivers/pcsp
> > sound/drivers/pcsp/pcsp.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_input.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_lib.c:#include "pcsp.h"
> > sound/drivers/pcsp/pcsp_mixer.c:#include "pcsp.h"
> > $ git grep -w i8253_lock sound/drivers/pcsp/
> > sound/drivers/pcsp/pcsp.h:static DEFINE_RAW_SPINLOCK(i8253_lock);
> > sound/drivers/pcsp/pcsp_input.c:        raw_spin_lock_irqsave(&i8253_lock,
> > flags
> > sound/drivers/pcsp/pcsp_input.c:       
> > raw_spin_unlock_irqrestore(&i8253_lock, 
> > sound/drivers/pcsp/pcsp_lib.c:          raw_spin_lock_irqsave(&i8253_lock,
> > flags
> > sound/drivers/pcsp/pcsp_lib.c:         
> > raw_spin_unlock_irqrestore(&i8253_lock, 
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_lock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_unlock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_lock(&i8253_lock);
> > sound/drivers/pcsp/pcsp_lib.c:  raw_spin_unlock(&i8253_lock);
> > 
> > Locks are great, everybody should have their own lock!
> > 
> > $ find . -name 8253pit.h
> > ./arch/powerpc/include/asm/8253pit.h
> > ./arch/alpha/include/asm/8253pit.h
> > $ cat arch/*/include/asm/8253pit.h
> > /*
> >  * 8253/8254 Programmable Interval Timer
> >  */
> > /*
> >  * 8253/8254 Programmable Interval Timer
> >  */
> > $
> > 
> > Eh...
> > 
> > $ git grep -w PCSPKR_PLATFORM 
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/mips/Kconfig:      select PCSPKR_PLATFORM
> > arch/powerpc/platforms/amigaone/Kconfig:        select PCSPKR_PLATFORM
> > drivers/input/misc/Kconfig:     depends on PCSPKR_PLATFORM
> > init/Kconfig:config PCSPKR_PLATFORM
> > sound/drivers/Kconfig:  depends on PCSPKR_PLATFORM && X86 &&
> > HIGH_RES_TIMERS
> > 
> > So the status is:
> > 
> >  Alpha:	There is no PCSPKR_PLATFORM so while a platform device is
> > 		being installed no drivers will be built.  I don't know
> > 		which Alpha platforms or even if all of Alpha should be
> > 		doing a PCSPKR_PLATFORM so I haven't even tried to sort
> > 		this.
> >  ARM:		No PC speaker supported, yeah :)
> >  PowerPC:	Should compile but the locking is wrong but only the 
> >    		AmigaOne platforms should be affected.
> The Kconfig dependencies cleanup patch for CHRP, PSERIES, etc. should
> also apply to the AmigaOne. Can you resend it with a fix for the
> AmigaOne, or should I send a patch?

I can sort that; it's easy enough.

> I'll check next week, if the PC speaker is still working on my AmigaOne.

I can't imagine that it's going to break - the code is sorta simple ;-)

One obscurity I noticed is this bit in the amigaone.dts:

        timer@40 {
                // Also adds pcspkr to platform devices.
                compatible = "pnpPNP,100";
                reg = <1 0x00000040 0x00000020>;
        };

Shouldn't that rather be something like the following?

        pcspeaker@61 {
                device_type = "sound";
                compatible = "pnpPNP,800";
                reg = <1 0x61 1>;
        };

pnpPNP,100 is the i8253 timer as I understand and pnpPNP,800 the PC speaker.
If you interpret pnpPNP,100 to imply the presence of a PC speaker you can't
express a system that has a i8253 but no PCspeaker in a DT so maybe
amigaone.dts and arch/powerpc/kernel/setup-common.c should be changed to
use pnpPNP,800 instead?

  Ralf

  reply	other threads:[~2011-06-03 18:01 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-01 18:04 [patch 00/14] Sort out i8253 and PC speaker locking and headers ralf
2011-06-01 18:04 ` [patch 01/14] i8253: Create <linux/i8253.h> and make all in kernel users to use it ralf
2011-06-09 11:16   ` [tip:timers/cleanup] i8253: Create linux/i8253.h and use it in all 8253 related files tip-bot for Ralf Baechle
2011-06-09 13:04   ` tip-bot for Ralf Baechle
2011-06-01 18:04 ` [patch 02/14] i8253: Unify all kernel declarations of i8253_lock in <linux/i8253.h> ralf
2011-06-01 18:04   ` ralf at linux-mips.org
2011-06-09 11:16   ` [tip:timers/cleanup] i8253: Unify all kernel declarations of i8253_lock tip-bot for Ralf Baechle
2011-06-09 13:04   ` tip-bot for Ralf Baechle
2011-06-01 18:04 ` [patch 03/14] i8253: Consolidate all kernel definitions of i8253_lock in lib/i8253-lock.c ralf
2011-06-09 11:17   ` [tip:timers/cleanup] i8253: Consolidate all kernel definitions of i8253_lock tip-bot for Ralf Baechle
2011-06-09 13:04   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 04/14] i8253: Make MIPS use the shared i8253_lock ralf
2011-06-01 18:05 ` [patch 05/14] i8253: Make x86 " ralf
2011-06-01 18:05 ` [patch 06/14] i8253: Make pcspkr input driver " ralf
2011-06-09 11:17   ` [tip:timers/cleanup] " tip-bot for Ralf Baechle
2011-06-09 13:05   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 07/14] i8253: Make pcsp sound " ralf
2011-06-01 18:05   ` ralf
2011-06-03  9:53   ` [alsa-devel] " Takashi Iwai
2011-06-03  9:53     ` Takashi Iwai
2011-06-09 11:18   ` [tip:timers/cleanup] " tip-bot for Ralf Baechle
2011-06-09 13:05   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 08/14] i8253: Alpha: Cleanup remaining users of i8253pit.h ralf
2011-06-09 11:19   ` [tip:timers/cleanup] alpha: i8253: " tip-bot for Ralf Baechle
2011-06-09 13:06   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 09/14] i8253: Alpha, PowerPC: Remove unused <asm/8253pit.h> header ralf
2011-06-01 18:05   ` ralf
2011-06-09  5:07   ` Benjamin Herrenschmidt
2011-06-09  5:07     ` Benjamin Herrenschmidt
2011-06-09 11:19   ` [tip:timers/cleanup] i8253: Alpha, PowerPC: Remove unused asm/8253pit.h tip-bot for Ralf Baechle
2011-06-09 13:07   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 10/14] i8253: x86: Consolidate definitions of global_clock_event ralf
2011-06-09 11:19   ` [tip:timers/cleanup] x86: i8253: " tip-bot for Ralf Baechle
2011-06-09 13:07   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 11/14] i8253: Consolidate definitions of PIT_LATCH ralf
2011-06-01 18:05   ` ralf at linux-mips.org
2011-06-01 18:05   ` ralf
2011-06-09 11:20   ` [tip:timers/cleanup] " tip-bot for Ralf Baechle
2011-06-09 13:07   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 12/14] i8253: Move remaining content and delete <asm/i8253.h> ralf
2011-06-01 18:05   ` ralf at linux-mips.org
2011-06-01 18:05   ` ralf
2011-06-02 11:29   ` Sergei Shtylyov
2011-06-02 11:29     ` Sergei Shtylyov
2011-06-09 11:20   ` [tip:timers/cleanup] i8253: Move remaining content and delete asm/i8253.h tip-bot for Ralf Baechle
2011-06-09 13:08   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 13/14] PCSPKR: Cleanup Kconfig dependencies ralf
2011-06-01 18:05   ` ralf
2011-06-09  5:09   ` Benjamin Herrenschmidt
2011-06-09  5:09     ` Benjamin Herrenschmidt
2011-06-09 11:21   ` [tip:timers/cleanup] pcspkr: " tip-bot for Ralf Baechle
2011-06-09 13:08   ` tip-bot for Ralf Baechle
2011-06-01 18:05 ` [patch 14/14] PCSPKR: MIPS: Make config dependencies finer grained ralf
2011-06-09 11:21   ` [tip:timers/cleanup] pcpskr: " tip-bot for Ralf Baechle
2011-06-09 13:09   ` tip-bot for Ralf Baechle
2011-06-02 19:11 ` [patch 00/14] Sort out i8253 and PC speaker locking and headers Gerhard Pircher
2011-06-03 18:00   ` Ralf Baechle [this message]
2011-06-03 18:00     ` Ralf Baechle
2011-06-03 19:22     ` Gerhard Pircher
2011-06-03 19:22       ` Gerhard Pircher

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=20110603180038.GA13239@linux-mips.org \
    --to=ralf@linux-mips.org \
    --cc=benh@kernel.crashing.org \
    --cc=gerhard_pircher@gmx.net \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=tglx@linutronix.de \
    /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.