* Re: pSeries_mach_cpu_die() question
From: Nathan Lynch @ 2006-06-02 6:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, Nathan Lynch
In-Reply-To: <1149225392.16202.52.camel@localhost.localdomain>
Benjamin Herrenschmidt wrote:
> While doing some autumn cleaning of the irq stuff in general and xics
> specifically, I found out that
> the low level pSeriesLP_cppr_info() is exported because
> pSeries_mach_cpu_die() calls it:
>
> static void pSeries_mach_cpu_die(void)
> {
> local_irq_disable();
> idle_task_exit();
> /* Some hardware requires clearing the CPPR, while other hardware does
> not
> * it is safe either way
> */
> pSeriesLP_cppr_info(0, 0);
> rtas_stop_self();
> /* Should never get here... */
> BUG();
> for(;;);
> }
>
> This leads to a few questions:
>
> - We always pass "0" as the CPU. Is that right ? I seems not, but maybe
> pHyp doesn't care and always assume the calling CPU ...
The cpu parameter is actually unused by in the lpar case:
void pSeriesLP_cppr_info(int n_cpu, u8 value)
{
unsigned long lpar_rc;
lpar_rc = plpar_cppr(value);
if (lpar_rc != H_SUCCESS)
panic("bad return code cppr - rc = %lx\n", lpar_rc);
}
> - xics has a xics_teardown_cpu() now, used by kexec, that does
> something very similar except that it passes the proper CPU number, and
> for secondary CPUs also does an EOI of any pending IPI (just in case). I
> think that could be used instead of the direct call to the low level
> pSeriesLP_* funciton (which I itend to unexport and rename anyway as
> part of my rework). Can whoever knows that code confirm ?
Sounds okay to me.
> - There is a comment about "some hardware....", what does it mean ? Is
> it ok to do it unconditionally ? I suppose so but heh...
The comment should be changed or removed really. We got away without
doing plpar_cppr() on the Power4 hypervisor but we found out it was
necessary when testing Power5. I think it's required by the
architecture regardless, and yes, it's safe on both platforms.
^ permalink raw reply
* Re: pSeries_mach_cpu_die() question
From: Benjamin Herrenschmidt @ 2006-06-02 6:26 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev list, Nathan Lynch
In-Reply-To: <20060602061929.GM8934@localdomain>
On Fri, 2006-06-02 at 01:19 -0500, Nathan Lynch wrote:
> The cpu parameter is actually unused by in the lpar case:
Ok, missed that :)
> > - xics has a xics_teardown_cpu() now, used by kexec, that does
> > something very similar except that it passes the proper CPU number, and
> > for secondary CPUs also does an EOI of any pending IPI (just in case). I
> > think that could be used instead of the direct call to the low level
> > pSeriesLP_* funciton (which I itend to unexport and rename anyway as
> > part of my rework). Can whoever knows that code confirm ?
>
> Sounds okay to me.
Ok. I'll call it with 0 for the "secondary" argument so it doesn't do
the additional EOI of the IPI in order to not change behaviour from the
current code. We can do differently in the future if we want.
> The comment should be changed or removed really. We got away without
> doing plpar_cppr() on the Power4 hypervisor but we found out it was
> necessary when testing Power5. I think it's required by the
> architecture regardless, and yes, it's safe on both platforms.
I'll remove the comment.
Thanks !
Cheers,
Ben.
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Zang Roy-r61911 @ 2006-06-02 7:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
> > +static void __init mpc7448_hpc2_map_io(void) {
> > + /* PCI IO mapping */
> > + io_block_mapping(MPC7448_HPC2_PCI_IO_BASE_VIRT,
> > + MPC7448_HPC2_PCI_IO_BASE_PHYS,
> 0x00800000, _PAGE_IO);
> > + /* Tsi108 CSR mapping */
> > + io_block_mapping(TSI108_CSR_ADDR_VIRT, TSI108_CSR_ADDR_PHYS,
> > + 0x100000, _PAGE_IO);
> > +
> > + /* PCI Config mapping */
> > + io_block_mapping(MPC7448_HPC2_PCI_CFG_BASE_VIRT,
> > + MPC7448_HPC2_PCI_CFG_BASE_PHYS,
> > + MPC7448_HPC2_PCI_CFG_SIZE, _PAGE_IO);
> > +
> > + tsi108_pci_cfg_base = MPC7448_HPC2_PCI_CFG_BASE_VIRT;
> > + /* NVRAM mapping */
> > + io_block_mapping(MPC7448_HPC2_NVRAM_BASE_ADDR,
> > + MPC7448_HPC2_NVRAM_BASE_ADDR,
> MPC7448_HPC2_NVRAM_SIZE,
> > + _PAGE_IO);
> > +}
>
> io_block_mapping is bad ! see endless discussions in the
> archives of why ... Use ioremap.
If I really need to use bat0 or bat1( I see that the general mmu use bat2 and bat3,
where should I set up them?
For tsi108 pci configuration access, I need to map 16Mbyte
physical address. If I do not use an extra bat, I can not get the correct
virtual address use ioremap.
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Benjamin Herrenschmidt @ 2006-06-02 7:32 UTC (permalink / raw)
To: Zang Roy-r61911
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
In-Reply-To: <9FCDBA58F226D911B202000BDBAD4673066CF5BE@zch01exm40.ap.freescale.net>
> If I really need to use bat0 or bat1( I see that the general mmu use bat2 and bat3,
> where should I set up them?
>
> For tsi108 pci configuration access, I need to map 16Mbyte
> physical address. If I do not use an extra bat, I can not get the correct
> virtual address use ioremap.
What do you mean ? An ioremap will work, it will give you any virtual
address, you just have to store that in a global instead of hard coding
it. Hard coded virtual addresses are bad. Especially for things like PCI
config space that really isn't performance sensitive.
Ben.
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Liu Dave-r63238 @ 2006-06-02 7:38 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', Zang Roy-r61911
Cc: linuxppc-dev list, Paul Mackerras, Alexandre.Bounine,
Yang Xin-Xin-r48390
> > If I really need to use bat0 or bat1( I see that the
> general mmu use
> > bat2 and bat3,
> > where should I set up them?
> >
> > For tsi108 pci configuration access, I need to map 16Mbyte
> > physical address. If I do not use an extra bat, I can not
> get the correct
> > virtual address use ioremap.
>
> What do you mean ? An ioremap will work, it will give you any
> virtual address, you just have to store that in a global
> instead of hard coding it. Hard coded virtual addresses are
> bad. Especially for things like PCI config space that really
> isn't performance sensitive.
>
I agree ben about PCI config space, It should use ioremap
to get virtual address. No performace indeed.
I think we need find the root cause that the ioremap failed.
Dave
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Zang Roy-r61911 @ 2006-06-02 7:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
>
> > If I really need to use bat0 or bat1( I see that the
> general mmu use
> > bat2 and bat3, where should I set up them?
> >
> > For tsi108 pci configuration access, I need to map 16Mbyte physical
> > address. If I do not use an extra bat, I can not get the correct
> > virtual address use ioremap.
>
> What do you mean ? An ioremap will work, it will give you any
> virtual address, you just have to store that in a global
> instead of hard coding it. Hard coded virtual addresses are
> bad. Especially for things like PCI config space that really
> isn't performance sensitive.
>
> Ben.
>
>
^ permalink raw reply
* Recall: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Zang Roy-r61911 @ 2006-06-02 7:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
Zang Roy-r61911 would like to recall the message, "[PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform".
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Zang Roy-r61911 @ 2006-06-02 8:05 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
> > If I really need to use bat0 or bat1( I see that the
> general mmu use
> > bat2 and bat3, where should I set up them?
> >
> > For tsi108 pci configuration access, I need to map 16Mbyte physical
> > address. If I do not use an extra bat, I can not get the correct
> > virtual address use ioremap.
>
> What do you mean ? An ioremap will work, it will give you any
> virtual address, you just have to store that in a global
> instead of hard coding it. Hard coded virtual addresses are
> bad. Especially for things like PCI config space that really
> isn't performance sensitive.
>
> Ben.
>
I had hoped to get the pci configure base address by ioremap, but failed.
the tsi108 register locates at 0xc000,0000 ~0xC001,0000. It is OK to
access them by ioremap. While pci configure access need to ioremap
0xfb00,0000 ~0xfc00,0000.
I traced my code, when I do ioremap, there is no bat match, I get the virtual
address from ioremap_bot. the init ioremap_bot is 0xfe000000. When I do
tsi108_csr_vir_base = ioremap(0xfb000000,0x1000000),
the ioremap_bot is 0xfdffe000 ( I get the serial port ioremap steal some space),
tsi108_csr_vir_base = 0xfdfee000, I can not access the configure space with this
address.
While if I use bat0 or bat1 to map my tsi108 register space, the ioremap_bot will keep
to 0xfe000000 until I do
tsi108_csr_vir_base = ioremap(0xfb000000,0x1000000)
then I get tsi108_csr_vir_base = 0xfd000000. Everything is OK.
Roy
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Benjamin Herrenschmidt @ 2006-06-02 8:17 UTC (permalink / raw)
To: Liu Dave-r63238
Cc: Alexandre.Bounine, linuxppc-dev list, Paul Mackerras,
Yang Xin-Xin-r48390
In-Reply-To: <9FCDBA58F226D911B202000BDBAD4673026FD937@zch01exm40.ap.freescale.net>
On Fri, 2006-06-02 at 15:38 +0800, Liu Dave-r63238 wrote:
> > > If I really need to use bat0 or bat1( I see that the
> > general mmu use
> > > bat2 and bat3,
> > > where should I set up them?
> > >
> > > For tsi108 pci configuration access, I need to map 16Mbyte
> > > physical address. If I do not use an extra bat, I can not
> > get the correct
> > > virtual address use ioremap.
> >
> > What do you mean ? An ioremap will work, it will give you any
> > virtual address, you just have to store that in a global
> > instead of hard coding it. Hard coded virtual addresses are
> > bad. Especially for things like PCI config space that really
> > isn't performance sensitive.
> >
>
> I agree ben about PCI config space, It should use ioremap
> to get virtual address. No performace indeed.
>
> I think we need find the root cause that the ioremap failed.
How much RAM do you have ? Maybe you are running out of virtual space ?
Ben.
^ permalink raw reply
* Re: Recall: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Benjamin Herrenschmidt @ 2006-06-02 8:17 UTC (permalink / raw)
To: Zang Roy-r61911
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
In-Reply-To: <9FCDBA58F226D911B202000BDBAD4673066CF61F@zch01exm40.ap.freescale.net>
On Fri, 2006-06-02 at 15:40 +0800, Zang Roy-r61911 wrote:
> Zang Roy-r61911 would like to recall the message, "[PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform".
Recalling of emails doesn't quite work :) Don't bother :)
Ben.
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Benjamin Herrenschmidt @ 2006-06-02 8:19 UTC (permalink / raw)
To: Zang Roy-r61911
Cc: linuxppc-dev list, Yang Xin-Xin-r48390, Paul Mackerras,
Alexandre.Bounine
In-Reply-To: <9FCDBA58F226D911B202000BDBAD4673066CF6DF@zch01exm40.ap.freescale.net>
On Fri, 2006-06-02 at 16:05 +0800, Zang Roy-r61911 wrote:
> > > If I really need to use bat0 or bat1( I see that the
> > general mmu use
> > > bat2 and bat3, where should I set up them?
> > >
> > > For tsi108 pci configuration access, I need to map 16Mbyte physical
> > > address. If I do not use an extra bat, I can not get the correct
> > > virtual address use ioremap.
> >
> > What do you mean ? An ioremap will work, it will give you any
> > virtual address, you just have to store that in a global
> > instead of hard coding it. Hard coded virtual addresses are
> > bad. Especially for things like PCI config space that really
> > isn't performance sensitive.
> >
> > Ben.
> >
>
> I had hoped to get the pci configure base address by ioremap, but failed.
> the tsi108 register locates at 0xc000,0000 ~0xC001,0000. It is OK to
> access them by ioremap. While pci configure access need to ioremap
> 0xfb00,0000 ~0xfc00,0000.
> I traced my code, when I do ioremap, there is no bat match, I get the virtual
> address from ioremap_bot. the init ioremap_bot is 0xfe000000. When I do
> tsi108_csr_vir_base = ioremap(0xfb000000,0x1000000),
> the ioremap_bot is 0xfdffe000 ( I get the serial port ioremap steal some space),
> tsi108_csr_vir_base = 0xfdfee000, I can not access the configure space with this
> address.
What happens if you try ?
> While if I use bat0 or bat1 to map my tsi108 register space, the ioremap_bot will keep
> to 0xfe000000 until I do
> tsi108_csr_vir_base = ioremap(0xfb000000,0x1000000)
> then I get tsi108_csr_vir_base = 0xfd000000. Everything is OK.
Both should work fine
Ben.
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Zang Roy-r61911 @ 2006-06-02 8:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev list, Paul Mackerras, Liu Dave-r63238,
Alexandre.Bounine, Yang Xin-Xin-r48390
> How much RAM do you have ? Maybe you are running out of
> virtual space ?
>
> Ben.
>
>
512MBytes
Roy
^ permalink raw reply
* RE: [PATCH/2.6.17-rc4 1/10] Powerpc: Add general support for mpc7 448h pc2 (Taiga) platform
From: Liu Dave-r63238 @ 2006-06-02 8:33 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', Zang Roy-r61911
Cc: linuxppc-dev list, Paul Mackerras, Alexandre.Bounine,
Yang Xin-Xin-r48390
> > I had hoped to get the pci configure base address by ioremap, but
> > failed. the tsi108 register locates at 0xc000,0000
> ~0xC001,0000. It is
> > OK to access them by ioremap. While pci configure access need to
> > ioremap 0xfb00,0000 ~0xfc00,0000. I traced my code, when I
> do ioremap,
> > there is no bat match, I get the virtual address from
> ioremap_bot. the
> > init ioremap_bot is 0xfe000000. When I do tsi108_csr_vir_base =
> > ioremap(0xfb000000,0x1000000), the ioremap_bot is
> 0xfdffe000 ( I get
> > the serial port ioremap steal some space), tsi108_csr_vir_base =
> > 0xfdfee000, I can not access the configure space with this address.
>
> What happens if you try ?
>
> > While if I use bat0 or bat1 to map my tsi108 register space, the
> > ioremap_bot will keep to 0xfe000000 until I do
> tsi108_csr_vir_base =
> > ioremap(0xfb000000,0x1000000) then I get tsi108_csr_vir_base =
> > 0xfd000000. Everything is OK.
>
> Both should work fine
>
> Ben.
>
The smallest BAT size of 74xx is 128KB, but the MPC7448_HPC2_NVRAM_SIZE
Is 0x8000 (32KB) you set in your code. I suggest you set the
MPC7448_HPC2_NVRAM_SIZE is 0x20000, try it.
Dave
^ permalink raw reply
* USB on MPC8349 with MPH controller
From: almoeli @ 2006-06-02 8:55 UTC (permalink / raw)
To: linuxppc-embedded
Hi,
im using kernel 2.6.16.18 with the ehci-fsl and a patch from this list
to get USB working.
So SCCR and SICRL register are set correctly.
The external connected PHY is a SMSC 3300-EZK which has an ULPI interface.
If I configure the USB controller of the MPC8349 to use the DR
controller, port 1 can be used with low, full and high speed devices
without any errors.
But if the USB controller is switched to MPH, the port 0 and port 1
cannot read the device descriptor.
Error message is:
fsl-usb2-mph: devpath1 ep0in 3strikes
fsl-usb2-mph: devpath1 ep0in 3strikes
fsl-usb2-mph: devpath1 ep0in 3strikes
usb1-1: device decriptor read/64, error -71
Does anyone know this problem or knows a solution?
Oliver
^ permalink raw reply
* MPC880 cannot use SCC4 together with FECs
From: Antonio Di Bacco @ 2006-06-02 9:21 UTC (permalink / raw)
To: linuxppc-embedded
This is a question for a Freescale guy or anyone that already used the MPC880.
I'm using an MPC880, both the FECs are used and SMC1, SMC2 and SCC3 are
currently being used too. Now I need to use the SCC4 as an asynchronous
serial port (only two pins) but my hardware engineer is saying that is not
possible because pins are missing. I cannot believe this, they told me that
RXD4 could use pins: PA11 or PD8 or PE25 but actually PA11 is used by the
first FEC for MII1-TXD0,
PD8 for MDC and PE25 for second FEC MII2-TXD3.
Is this correct?
Bye,
Antonio.
^ permalink raw reply
* snd-aoa: using feature calls for GPIOs
From: Johannes Berg @ 2006-06-01 18:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
Hi,
The following patch ought to add the possibility of using feature calls
for the GPIOs instead of platform functions. Note that it doesn't
actually hook up the new gpio functions, that needs to be done in the
fabric.
However, the whole thing doesn't work for me yet. In fact, as soon as I
try using this patch, I need to reboot my computer to make the tas react
again to i2c commands. Very strange.
I looked into snd-powermac, and it does very weird things with the
GPIOs. I notice, for example, the following code:
base = (u32 *)get_property(node, "audio-gpio-active-state", NULL);
if (base) {
gp->active_state = *base;
gp->active_val = (*base) ? 0x5 : 0x4;
gp->inactive_val = (*base) ? 0x4 : 0x5;
As far as I can see that's bogus. The 0x4/0x5 values aren't ever
actually written to the register, they are just the software values
Darwin uses for unmute/mute (or was it reset/run?).
I don't see where this comes from, looking at Darwin gives you that all
GPIO writes in PlatformInterfaceGPIO_Mapped are either 0 or 1 depending
on the activestate value and using assertGPIO/negateGPIO. I think the
4/5 comes from confusing the kGPIO_* values with what is actually
written to the register.
Also, snd-powermac says: "Try to find the active state, default to 0 !".
That isn't what Darwin does, it defaults to 1.
Anyway, I'm sure I'm doing something stupid in there and just can't find
it, I even 'disassembled' all the platform functions I have to see what
happens in them, and they also write 0 or 1 just like the code below...
Maybe I got some offsets wrong?
--- snd-aoa.orig/aoa.h 2006-06-01 20:29:56.252070199 +0200
+++ snd-aoa/aoa.h 2006-06-01 20:31:03.972070199 +0200
@@ -125,6 +125,7 @@ extern int aoa_snd_ctl_add(struct snd_kc
/* GPIO stuff */
extern struct gpio_methods *pmf_gpio_methods;
+extern struct gpio_methods *ftr_gpio_methods;
/* extern struct gpio_methods *map_gpio_methods; */
#endif /* __AOA_H */
--- snd-aoa.orig/core/Makefile 2006-06-01 20:29:56.252070199 +0200
+++ snd-aoa/core/Makefile 2006-06-01 20:31:03.972070199 +0200
@@ -1,4 +1,5 @@
obj-$(CONFIG_SND_AOA) += snd-aoa.o
snd-aoa-objs := snd-aoa-core.o \
snd-aoa-alsa.o \
- snd-aoa-gpio-pmf.o
+ snd-aoa-gpio-pmf.o \
+ snd-aoa-gpio-feature.o
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ snd-aoa/core/snd-aoa-gpio-feature.c 2006-06-01 20:31:03.992070199 +0200
@@ -0,0 +1,322 @@
+/*
+ * Apple Onboard Audio feature call GPIO control
+ *
+ * Copyright 2006 Johannes Berg <johannes@sipsolutions.net>
+ *
+ * GPL v2, can be found in COPYING.
+ */
+
+#include <asm/pmac_feature.h>
+#include <linux/interrupt.h>
+#include "../aoa.h"
+
+static int headphone_mute_gpio;
+static int amp_mute_gpio;
+static int lineout_mute_gpio;
+static int hw_reset_gpio;
+static int lineout_detect_gpio;
+static int headphone_detect_gpio;
+static int linein_detect_gpio;
+
+static int headphone_mute_gpio_activestate;
+static int amp_mute_gpio_activestate;
+static int lineout_mute_gpio_activestate;
+static int hw_reset_gpio_activestate;
+static int lineout_detect_gpio_activestate;
+static int headphone_detect_gpio_activestate;
+static int linein_detect_gpio_activestate;
+
+static int lineout_detect_irq;
+static int linein_detect_irq;
+static int headphone_detect_irq;
+
+static void get_gpio(char *name, int *gpioptr, int *gpioactiveptr)
+{
+ struct device_node *np;
+ u32 *reg;
+
+ *gpioptr = -1;
+
+ np = of_find_node_by_name(NULL, name);
+ if (!np)
+ return;
+
+ reg = (u32 *)get_property(np, "reg", NULL);
+ if (!reg)
+ return;
+
+ *gpioptr = *reg;
+
+ /* this is a hack, usually the GPIOs 'reg' property
+ * should have the offset based from the GPIO space
+ * which is at 0x50, but apparently not always... */
+ if (*gpioptr < 0x50)
+ *gpioptr += 0x50;
+
+ reg = (u32 *)get_property(np, "audio-gpio-active-state", NULL);
+ if (!reg)
+ *gpioactiveptr = 1;
+ else
+ *gpioactiveptr = *reg;
+
+ printk(KERN_DEBUG "gpio %s = %d (active = %d)\n", name, *gpioptr, *gpioactiveptr);
+}
+
+static void get_irq(char *name, int *irqptr)
+{
+ struct device_node *np;
+
+ *irqptr = -1;
+ np = of_find_node_by_name(NULL, name);
+ if (!np)
+ return;
+ if (np->n_intrs != 1)
+ return;
+ *irqptr = np->intrs[0].line;
+
+ printk(KERN_DEBUG "got %s irq = %d\n", name, *irqptr);
+}
+
+#define SWITCH_GPIO(name, on) \
+ ((on)?(name##_gpio_activestate==0?0:1):(name##_gpio_activestate==0?1:0))
+
+#define FTR_GPIO(name, bit) \
+static void ftr_gpio_set_##name(struct gpio_runtime *rt, int on)\
+{ \
+ if (unlikely(!rt)) return; \
+ \
+ if (name##_mute_gpio < 0) \
+ return; \
+ \
+ pmac_call_feature(PMAC_FTR_WRITE_GPIO, NULL, \
+ name##_mute_gpio, \
+ SWITCH_GPIO(name##_mute, on)); \
+ \
+ rt->implementation_private &= ~(1<<bit); \
+ rt->implementation_private |= (!!on << bit); \
+} \
+static int ftr_gpio_get_##name(struct gpio_runtime *rt) \
+{ \
+ if (unlikely(!rt)) return 0; \
+ return (rt->implementation_private>>bit)&1; \
+}
+
+FTR_GPIO(headphone, 0);
+FTR_GPIO(amp, 1);
+FTR_GPIO(lineout, 2);
+
+static void ftr_gpio_set_hw_reset(struct gpio_runtime *rt, int on)
+{
+ if (unlikely(!rt)) return;
+ if (hw_reset_gpio < 0)
+ return;
+
+ pmac_call_feature(PMAC_FTR_WRITE_GPIO, NULL,
+ hw_reset_gpio, SWITCH_GPIO(hw_reset, on));
+}
+
+static void ftr_gpio_all_amps_off(struct gpio_runtime *rt)
+{
+ int saved;
+
+ if (unlikely(!rt)) return;
+ saved = rt->implementation_private;
+ ftr_gpio_set_headphone(rt, 0);
+ ftr_gpio_set_amp(rt, 0);
+ ftr_gpio_set_lineout(rt, 0);
+ rt->implementation_private = saved;
+}
+
+static void ftr_gpio_all_amps_restore(struct gpio_runtime *rt)
+{
+ int s;
+
+ if (unlikely(!rt)) return;
+ s = rt->implementation_private;
+ ftr_gpio_set_headphone(rt, (s>>0)&1);
+ ftr_gpio_set_amp(rt, (s>>1)&1);
+ ftr_gpio_set_lineout(rt, (s>>2)&1);
+}
+
+static void ftr_handle_notify(void *data)
+{
+ struct gpio_notification *notif = data;
+
+ mutex_lock(¬if->mutex);
+ if (notif->notify)
+ notif->notify(notif->data);
+ mutex_unlock(¬if->mutex);
+}
+
+static void ftr_gpio_init(struct gpio_runtime *rt)
+{
+ get_gpio("headphone-mute", &headphone_mute_gpio,
+ &headphone_mute_gpio_activestate);
+ get_gpio("amp-mute", &_mute_gpio,
+ &_mute_gpio_activestate);
+ get_gpio("lineout-mute", &lineout_mute_gpio,
+ &lineout_mute_gpio_activestate);
+ get_gpio("hw-reset", &hw_reset_gpio,
+ &hw_reset_gpio_activestate);
+ get_gpio("headphone-detect", &headphone_detect_gpio,
+ &headphone_detect_gpio_activestate);
+ get_gpio("lineout-detect", &lineout_detect_gpio,
+ &lineout_detect_gpio_activestate);
+ get_gpio("linein-detect", &linein_detect_gpio,
+ &linein_detect_gpio_activestate);
+
+ get_irq("headphone-detect", &headphone_detect_irq);
+ get_irq("lineout-detect", &lineout_detect_irq);
+ get_irq("linein-detect", &linein_detect_irq);
+
+ ftr_gpio_all_amps_off(rt);
+ rt->implementation_private = 0;
+ INIT_WORK(&rt->headphone_notify.work, ftr_handle_notify, &rt->headphone_notify);
+ INIT_WORK(&rt->line_in_notify.work, ftr_handle_notify, &rt->line_in_notify);
+ INIT_WORK(&rt->line_out_notify.work, ftr_handle_notify, &rt->line_out_notify);
+ mutex_init(&rt->headphone_notify.mutex);
+ mutex_init(&rt->line_in_notify.mutex);
+ mutex_init(&rt->line_out_notify.mutex);
+}
+
+static void ftr_gpio_exit(struct gpio_runtime *rt)
+{
+ ftr_gpio_all_amps_off(rt);
+ rt->implementation_private = 0;
+ if (rt->headphone_notify.notify)
+ free_irq(headphone_detect_irq, &rt->headphone_notify);
+ if (rt->line_in_notify.gpio_private)
+ free_irq(linein_detect_irq, &rt->line_in_notify);
+ if (rt->line_out_notify.gpio_private)
+ free_irq(lineout_detect_irq, &rt->line_out_notify);
+ cancel_delayed_work(&rt->headphone_notify.work);
+ cancel_delayed_work(&rt->line_in_notify.work);
+ cancel_delayed_work(&rt->line_out_notify.work);
+ flush_scheduled_work();
+ mutex_destroy(&rt->headphone_notify.mutex);
+ mutex_destroy(&rt->line_in_notify.mutex);
+ mutex_destroy(&rt->line_out_notify.mutex);
+}
+
+irqreturn_t ftr_handle_notify_irq(int xx, void *data, struct pt_regs *regs)
+{
+ struct gpio_notification *notif = data;
+
+ schedule_work(¬if->work);
+
+ return IRQ_HANDLED;
+}
+
+static int ftr_set_notify(struct gpio_runtime *rt,
+ enum notify_type type,
+ notify_func_t notify,
+ void *data)
+{
+ struct gpio_notification *notif;
+ notify_func_t old;
+ int irq;
+ char *name;
+ int err = -EBUSY;
+
+ switch (type) {
+ case AOA_NOTIFY_HEADPHONE:
+ notif = &rt->headphone_notify;
+ name = "headphone-detect";
+ irq = headphone_detect_irq;
+ break;
+ case AOA_NOTIFY_LINE_IN:
+ notif = &rt->line_in_notify;
+ name = "linein-detect";
+ irq = linein_detect_irq;
+ break;
+ case AOA_NOTIFY_LINE_OUT:
+ notif = &rt->line_out_notify;
+ name = "lineout-detect";
+ irq = lineout_detect_irq;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (irq == -1)
+ return -ENODEV;
+
+ mutex_lock(¬if->mutex);
+
+ old = notif->notify;
+
+ if (!old && !notify) {
+ err = 0;
+ goto out_unlock;
+ }
+
+ if (old && notify) {
+ if (old == notify && notif->data == data)
+ err = 0;
+ goto out_unlock;
+ }
+
+ if (old && !notify) {
+ free_irq(irq, notif);
+ }
+ if (!old && notify) {
+ request_irq(irq, ftr_handle_notify_irq, 0, name, notif);
+ }
+ notif->notify = notify;
+ notif->data = data;
+
+ err = 0;
+ out_unlock:
+ mutex_unlock(¬if->mutex);
+ return err;
+}
+
+static int ftr_get_detect(struct gpio_runtime *rt,
+ enum notify_type type)
+{
+ int gpio, ret, active;
+
+ switch (type) {
+ case AOA_NOTIFY_HEADPHONE:
+ gpio = headphone_detect_gpio;
+ active = headphone_detect_gpio_activestate;
+ break;
+ case AOA_NOTIFY_LINE_IN:
+ gpio = linein_detect_gpio;
+ active = linein_detect_gpio_activestate;
+ break;
+ case AOA_NOTIFY_LINE_OUT:
+ gpio = lineout_detect_gpio;
+ active = lineout_detect_gpio_activestate;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (gpio == -1)
+ return -ENODEV;
+
+ ret = pmac_call_feature(PMAC_FTR_READ_GPIO, NULL, gpio, 0);
+ if (ret < 0)
+ return ret;
+ return ((ret >> 1) & 1) == active;
+}
+
+static struct gpio_methods methods = {
+ .init = ftr_gpio_init,
+ .exit = ftr_gpio_exit,
+ .all_amps_off = ftr_gpio_all_amps_off,
+ .all_amps_restore = ftr_gpio_all_amps_restore,
+ .set_headphone = ftr_gpio_set_headphone,
+ .set_speakers = ftr_gpio_set_amp,
+ .set_lineout = ftr_gpio_set_lineout,
+ .set_hw_reset = ftr_gpio_set_hw_reset,
+ .get_headphone = ftr_gpio_get_headphone,
+ .get_speakers = ftr_gpio_get_amp,
+ .get_lineout = ftr_gpio_get_lineout,
+ .set_notify = ftr_set_notify,
+ .get_detect = ftr_get_detect,
+};
+
+struct gpio_methods *ftr_gpio_methods = &methods;
+EXPORT_SYMBOL_GPL(ftr_gpio_methods);
^ permalink raw reply
* Re: [PATCH] [2.6.18] U4 DART improvements
From: Segher Boessenkool @ 2006-06-02 12:44 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev, paulus
In-Reply-To: <20060602040426.GA28528@pb15.lixom.net>
Hi Olof,
Looks good. One request:
> +static inline void dart_tlb_invalidate_one(unsigned long bus_rpn)
> +{
> + unsigned int reg;
> + unsigned int l, limit;
> +
> + reg = DART_CNTL_U4_ENABLE | DART_CNTL_U4_IONE |
> + (bus_rpn & DART_CNTL_U4_IONE_MASK);
> + DART_OUT(DART_CNTL, reg);
> + mb();
Could you please comment the memory barriers, to say exactly _why_ a
certain barrier is needed? I can't see why wmb() wouldn't work here,
for example (note I'm not saying it would -- I just don't see why it
wouldn't).
Same goes for every single memory barrier in the whole kernel source
code, but I have to start somewhere, heh.
Segher
^ permalink raw reply
* Re: [Alsa-devel] [RFC 2/8] snd-aoa: add aoa core
From: Takashi Iwai @ 2006-06-02 13:42 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20060601115845.923706000@sipsolutions.net>
At Thu, 01 Jun 2006 13:58:46 +0200,
Johannes Berg wrote:
>
> --- /dev/null
> +++ b/sound/aoa/core/snd-aoa-core.c
> @@ -0,0 +1,153 @@
> +/*
> + * Apple Onboard Audio driver core
> + *
> + * Copyright 2006 Johannes Berg <johannes@sipsolutions.net>
> + *
> + * GPL v2, can be found in COPYING.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include "../aoa.h"
> +#include "snd-aoa-alsa.h"
> +
> +MODULE_DESCRIPTION("Apple Onboard Audio Sound Driver");
> +MODULE_AUTHOR("Johannes Berg <johannes@sipsolutions.net>");
> +MODULE_LICENSE("GPL");
> +
> +/* We allow only one fabric. This simplifies things,
> + * and more don't really make that much sense */
> +static struct aoa_fabric *fabric;
> +static LIST_HEAD(codec_list);
> +
> +static void attach_codec_to_fabric(struct aoa_codec *c)
Doesn't this need to return an error?
I'm afraid that module count is unblanced in the error path of
aoa_corec_[un]register().
> +{
> + int err;
> +
> + if (!try_module_get(c->owner))
> + return;
> + /* found_codec has to be assigned */
> + err = -ENOENT;
> + if (fabric->found_codec)
> + err = fabric->found_codec(c);
> + if (err) {
> + module_put(c->owner);
> + printk("snd-aoa: fabric didn't like codec %s\n", c->name);
Add KERN_* prefix.
> + return;
> + }
> + c->fabric = fabric;
> +
> + err = 0;
> + if (c->init)
> + err = c->init(c);
> + if (err) {
> + printk("snd-aoa: codec %s didn't init\n", c->name);
Ditto.
> --- /dev/null
> +++ b/sound/aoa/core/snd-aoa-gpio-pmf.c
(snip)
> +static void pmf_gpio_init(struct gpio_runtime *rt)
> +{
> + pmf_gpio_all_amps_off(rt);
> + rt->implementation_private = 0;
> + INIT_WORK(&rt->headphone_notify.work, pmf_handle_notify, &rt->headphone_notify);
> + INIT_WORK(&rt->line_in_notify.work, pmf_handle_notify, &rt->line_in_notify);
> + INIT_WORK(&rt->line_out_notify.work, pmf_handle_notify, &rt->line_out_notify);
Too long lines.
> + mutex_init(&rt->headphone_notify.mutex);
> + mutex_init(&rt->line_in_notify.mutex);
> + mutex_init(&rt->line_out_notify.mutex);
> +}
> +
> +static void pmf_gpio_exit(struct gpio_runtime *rt)
> +{
> + pmf_gpio_all_amps_off(rt);
> + rt->implementation_private = 0;
> + if (rt->headphone_notify.gpio_private)
> + pmf_unregister_irq_client(rt->headphone_notify.gpio_private);
> + if (rt->line_in_notify.gpio_private)
> + pmf_unregister_irq_client(rt->line_in_notify.gpio_private);
> + if (rt->line_out_notify.gpio_private)
> + pmf_unregister_irq_client(rt->line_out_notify.gpio_private);
Don't need kfree(gpio_private)?
> --- /dev/null
> +++ b/sound/aoa/core/snd-aoa-alsa.c
(snip)
> +int aoa_alsa_init(char *name, struct module *mod)
> +{
> + struct snd_card *alsa_card;
> + int err;
> +
> + if (aoa_card)
> + /* cannot be EEXIST due to usage in aoa_fabric_register */
> + return -EBUSY;
> +
> + alsa_card = snd_card_new(index, name, mod, sizeof(struct aoa_card));
> + if (!alsa_card)
> + return -ENOMEM;
> + aoa_card = alsa_card->private_data;
> + aoa_card->alsa_card = alsa_card;
> + strlcpy(alsa_card->driver, "AppleOnbdAudio", sizeof(alsa_card->driver)-1);
> + strlcpy(alsa_card->shortname, name, sizeof(alsa_card->shortname)-1);
> + strlcpy(alsa_card->longname, name, sizeof(alsa_card->longname)-1);
> + strlcpy(alsa_card->mixername, name, sizeof(alsa_card->mixername)-1);
Pass sizeof() without -1. strlcpy() takes the size of the buffer
including nul-terminator.
Takashi
^ permalink raw reply
* Re: [Alsa-devel] [RFC 2/8] snd-aoa: add aoa core
From: Johannes Berg @ 2006-06-02 13:46 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <s5hverj4qsd.wl%tiwai@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]
On Fri, 2006-06-02 at 15:42 +0200, Takashi Iwai wrote:
> > +static void attach_codec_to_fabric(struct aoa_codec *c)
>
> Doesn't this need to return an error?
> I'm afraid that module count is unblanced in the error path of
> aoa_corec_[un]register().
Not sure, I'll have to take a look.
> > + printk("snd-aoa: fabric didn't like codec %s\n", c->name);
>
> Add KERN_* prefix.
Uh, right.
> > + rt->implementation_private = 0;
> > + INIT_WORK(&rt->headphone_notify.work, pmf_handle_notify, &rt->headphone_notify);
> > + INIT_WORK(&rt->line_in_notify.work, pmf_handle_notify, &rt->line_in_notify);
> > + INIT_WORK(&rt->line_out_notify.work, pmf_handle_notify, &rt->line_out_notify);
>
> Too long lines.
Heh, I knew I'd get this at lots of places, I'll work on it.
> > + pmf_gpio_all_amps_off(rt);
> > + rt->implementation_private = 0;
> > + if (rt->headphone_notify.gpio_private)
> > + pmf_unregister_irq_client(rt->headphone_notify.gpio_private);
> > + if (rt->line_in_notify.gpio_private)
> > + pmf_unregister_irq_client(rt->line_in_notify.gpio_private);
> > + if (rt->line_out_notify.gpio_private)
> > + pmf_unregister_irq_client(rt->line_out_notify.gpio_private);
>
> Don't need kfree(gpio_private)?
Indeed, forgot that.
> > + strlcpy(alsa_card->driver, "AppleOnbdAudio", sizeof(alsa_card->driver)-1);
> > + strlcpy(alsa_card->shortname, name, sizeof(alsa_card->shortname)-1);
> > + strlcpy(alsa_card->longname, name, sizeof(alsa_card->longname)-1);
> > + strlcpy(alsa_card->mixername, name, sizeof(alsa_card->mixername)-1);
>
> Pass sizeof() without -1. strlcpy() takes the size of the buffer
> including nul-terminator.
Yeah, I was confused about the API. Will fix those too.
Thanks,
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 793 bytes --]
^ permalink raw reply
* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Takashi Iwai @ 2006-06-02 13:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20060601115846.852128000@sipsolutions.net>
At Thu, 01 Jun 2006 13:58:47 +0200,
Johannes Berg wrote:
>
> --- /dev/null
> +++ b/sound/aoa/soundbus/sysfs.c
> +static ssize_t
> +compatible_show (struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct of_device *of;
> + char *compat;
> + int cplen;
> + int length = 0;
> +
> + of = &to_soundbus_device (dev)->ofdev;
> + compat = (char *) get_property(of->node, "compatible", &cplen);
> + if (!compat) {
> + *buf = '\0';
> + return 0;
> + }
> + while (cplen > 0) {
> + int l;
> + length += sprintf (buf, "%s\n", compat);
> + buf += length;
This seems wrong. The above should be
l = sprintf(buf + length, "%s\n", compat);
length += l;
buf += l;
> + l = strlen (compat) + 1;
> + compat += l;
> + cplen -= l;
> + }
> +
> + return length;
> +}
> +
> +static ssize_t modalias_show (struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct soundbus_dev *sdev = to_soundbus_device(dev);
> + struct of_device *of = &sdev->ofdev;
> + int length;
> +
> + if (strlen(sdev->modalias)) {
> + length = snprintf (buf, 34, "%s\n", sdev->modalias);
sizeof(sdev->modalias) + 2? But still strange if modalias has no
nul-end.
> + } else {
> + length = sprintf (buf, "of:N%sT%s\n", of->node->name, of->node->type);
> + }
Remove braces for single-line if (and too long).
Takashi
^ permalink raw reply
* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Johannes Berg @ 2006-06-02 14:07 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <s5hu0734qay.wl%tiwai@suse.de>
[-- Attachment #1: Type: text/plain, Size: 562 bytes --]
On Fri, 2006-06-02 at 15:53 +0200, Takashi Iwai wrote:
> > + int l;
> > + length += sprintf (buf, "%s\n", compat);
> > + buf += length;
>
> This seems wrong. The above should be
Heh, no idea where I copied that from but I sure didn't write it myself.
Should be more careful next time I guess :)
> > + if (strlen(sdev->modalias)) {
> > + length = snprintf (buf, 34, "%s\n", sdev->modalias);
>
> sizeof(sdev->modalias) + 2? But still strange if modalias has no
> nul-end.
Hm, how would I handle that if modalias has no nul?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 793 bytes --]
^ permalink raw reply
* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Takashi Iwai @ 2006-06-02 14:21 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <1149257225.11092.22.camel@johannes.berg>
At Fri, 02 Jun 2006 16:07:05 +0200,
Johannes Berg wrote:
>
> On Fri, 2006-06-02 at 15:53 +0200, Takashi Iwai wrote:
> > > + if (strlen(sdev->modalias)) {
> > > + length = snprintf (buf, 34, "%s\n", sdev->modalias);
> >
> > sizeof(sdev->modalias) + 2? But still strange if modalias has no
> > nul-end.
>
> Hm, how would I handle that if modalias has no nul?
Well, I'm not sure whether modalias is always nul-terminated.
If yes (I hope so), the above could be simply sprintf().
Otherwise (if not nul-terminated), I'd implement like:
strlcpy(buf, sdev->modalias, sizeof(sdev->modalias) + 1);
strcpy(buf, "\n");
lenth = strlen(buf);
Takashi
^ permalink raw reply
* Re: [Alsa-devel] [RFC 4/8] snd-aoa: add i2sbus
From: Takashi Iwai @ 2006-06-02 14:23 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20060601115847.420788000@sipsolutions.net>
At Thu, 01 Jun 2006 13:58:48 +0200,
Johannes Berg wrote:
>
> --- /dev/null
> +++ b/sound/aoa/soundbus/i2sbus/i2sbus-pcm.c
> +static int clock_and_divisors(int mclk, int sclk, int rate, int *out)
> +{
> + /* sclk must be derived from mclk! */
> + if (mclk % sclk)
> + return -1;
> + /* derive sclk register value */
> + if (i2s_sf_sclkdiv(mclk / sclk, out))
> + return -1;
> +
> + if (I2S_CLOCK_SPEED_18MHz % rate == 0) {
> + if ((I2S_CLOCK_SPEED_18MHz / rate) % mclk == 0) {
Equivalent with "I2S_CLOCK_SPEED_18MHZ % (rate * mclk) == 0" ?
> +static int i2sbus_pcm_open(struct i2sbus_dev *i2sdev, int in)
> +{
(snip)
> + list_for_each_entry(cii, &sdev->codec_list, list) {
> + if (cii->codec->open) {
> + err = cii->codec->open(cii, pi->substream);
> + if (err) {
> + result = err;
> + goto out_unlock;
What happens if the first code is opened but fail the secondary?
No need to close the first?
> +static snd_pcm_uframes_t i2sbus_pcm_pointer(struct i2sbus_dev *i2sdev, int in)
> +{
> + struct pcm_info *pi;
> + u32 fc;
> +
> + get_pcm_info(i2sdev, in, &pi, NULL);
> +
> + fc = in_le32(&i2sdev->intfregs->frame_count);
> + fc = fc - pi->frame_count;
> +
> + return (bytes_to_frames(pi->substream->runtime,
> + pi->current_period *
> + snd_pcm_lib_period_bytes(pi->substream)) + fc) % pi->substream->runtime->buffer_size;
> +}
> +
> +static inline void handle_interrupt(struct i2sbus_dev *i2sdev, int in)
> +{
> + struct pcm_info *pi;
> + u32 fc;
> + u32 delta;
> +
> + spin_lock(&i2sdev->low_lock);
> + get_pcm_info(i2sdev, in, &pi, NULL);
> + if (!pi->substream) {
> + printk(KERN_INFO "i2sbus: got %s irq while not active!\n",
> + in ? "rx" : "tx");
> + goto out_unlock;
> + }
> +
> + fc = in_le32(&i2sdev->intfregs->frame_count);
> + /* a counter overflow does not change the calculation. */
> + delta = fc - pi->frame_count;
> +
> + /* update current_period */
> + while (delta >= pi->substream->runtime->period_size) {
> + pi->current_period++;
> + delta = delta - pi->substream->runtime->period_size;
> + }
> +
> + if (unlikely(delta)) {
> + /* Some interrupt came late, so check the dbdma.
> + * This special case exists to syncronize the frame_count with the
> + * dbdma transfers, but is hit every once in a while. */
> + int period;
> +
> + period = (in_le32(&pi->dbdma->cmdptr) - pi->dbdma_ring.bus_cmd_start) / sizeof(struct dbdma_cmd);
> + pi->current_period = pi->current_period % pi->substream->runtime->periods;
> +
> + while (pi->current_period != period) {
> + pi->current_period = (pi->current_period + 1) % pi->substream->runtime->periods;
> + /* Set delta to zero, as the frame_count value is too high (otherwise the code path
> + * will not be executed).
> + * This is to correct the fact that the frame_count is too low at the beginning
> + * due to the dbdma's buffer. */
> + delta = 0;
Too long lines...
> +/* FIXME: this function needs an error handling strategy with labels */
> +int
> +i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card,
> + struct codec_info *ci, void *data)
> +{
> + int err, in = 0, out = 0;
> + struct transfer_info *tmp;
> + struct i2sbus_dev *i2sdev = soundbus_dev_to_i2sbus_dev(dev);
> + struct codec_info_item *cii;
> +
> + if (!dev->pcmname || dev->pcmid == -1) {
> + printk(KERN_ERR "i2sbus: pcm name and id must be set!\n");
No error return?
> + /* well, we really should support scatter/gather DMA */
> + /* FIXME FIXME FIXME: If this fails, we BUG() when the alsa layer
> + * later tries to allocate memory. Apparently we should be setting
> + * some device pointer for that ...
> + */
> + snd_pcm_lib_preallocate_pages_for_all(
> + dev->pcm, SNDRV_DMA_TYPE_DEV,
> + snd_dma_pci_data(macio_get_pci_dev(i2sdev->macio)),
> + 64 * 1024, 64 * 1024);
Is the comment true? Yes, you have to set the device pointer via
snd_pcm_lib_preallocate*(). But it must be OK even if preallocate
fails.
> --- /dev/null
> +++ b/sound/aoa/soundbus/i2sbus/i2sbus-core.c
> +static int alloc_dbdma_descriptor_ring(struct i2sbus_dev *i2sdev,
> + struct dbdma_command_mem *r,
> + int numcmds)
> +{
> + /* one more for rounding */
> + r->size = (numcmds+1) * sizeof(struct dbdma_cmd);
> + /* We use the PCI APIs for now until the generic one gets fixed
> + * enough or until we get some macio-specific versions
> + */
> + r->space = pci_alloc_consistent(macio_get_pci_dev(i2sdev->macio),
> + r->size,
> + &r->bus_addr);
Better to use dma_alloc_coherent(). pci_alloc_consistent() implies
GFP_ATOMIC.
> +static irqreturn_t i2sbus_bus_intr(int irq, void *devid, struct pt_regs *regs)
> +{
> + struct i2sbus_dev *dev = devid;
> + u32 intreg;
> +
> + spin_lock(&dev->low_lock);
> + intreg = in_le32(&dev->intfregs->intr_ctl);
> +
> + printk(KERN_INFO "i2sbus: interrupt, intr reg is 0x%x!\n", intreg);
Should this be really always printed?
Takashi
^ permalink raw reply
* Re: [Alsa-devel] [RFC 5/8] snd-aoa: add codecs
From: Takashi Iwai @ 2006-06-02 14:31 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20060601115848.075177000@sipsolutions.net>
At Thu, 01 Jun 2006 13:58:49 +0200,
Johannes Berg wrote:
>
> --- /dev/null
> +++ b/sound/aoa/codecs/snd-aoa-codec-tas-gain-table.h
> @@ -0,0 +1,209 @@
> +/*
> + This is the program used to generate below table.
> +
> +#include <stdio.h>
> +#include <math.h>
> +int main() {
> + int dB2;
> + printf("/" "* This file is only included exactly once!\n");
> + printf(" *\n");
> + printf(" * If they'd only tell us that generating this table was\n");
> + printf(" * as easy as calculating\n");
> + printf(" * hwvalue = 1048576.0*exp(0.057564628*dB*2)\n");
> + printf(" * :) *" "/\n");
> + printf("static int tas_gaintable[] = {\n");
> + printf(" 0x000000, /" "* -infinity dB *" "/\n");
> + for (dB2=-140;dB2<=36;dB2++)
> + printf(" 0x%.6x, /" "* %-02.1f dB *" "/\n", (int)(1048576.0*exp(0.057564628*dB2)), dB2/2.0);
> + printf("};\n\n");
> +}
FYI: We're currently implementing a dB conversion. So, this will be
unneeded in future.
But I'll included this as it is now at committing, and fix the dB
things later.
Takashi
^ permalink raw reply
* Re: [Alsa-devel] [RFC 3/8] snd-aoa: add soundbus
From: Andreas Schwab @ 2006-06-02 14:41 UTC (permalink / raw)
To: Takashi Iwai; +Cc: linuxppc-dev, Johannes Berg, alsa-devel
In-Reply-To: <s5hslmn4p00.wl%tiwai@suse.de>
Takashi Iwai <tiwai@suse.de> writes:
> At Fri, 02 Jun 2006 16:07:05 +0200,
> Johannes Berg wrote:
>>
>> On Fri, 2006-06-02 at 15:53 +0200, Takashi Iwai wrote:
>> > > + if (strlen(sdev->modalias)) {
>> > > + length = snprintf (buf, 34, "%s\n", sdev->modalias);
>> >
>> > sizeof(sdev->modalias) + 2? But still strange if modalias has no
>> > nul-end.
>>
>> Hm, how would I handle that if modalias has no nul?
>
> Well, I'm not sure whether modalias is always nul-terminated.
If modalias is not nul-terminated then strlen will already fail.
> Otherwise (if not nul-terminated), I'd implement like:
>
> strlcpy(buf, sdev->modalias, sizeof(sdev->modalias) + 1);
> strcpy(buf, "\n");
strcat?
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox