Hi Dominik,
Today's linux-next merge of the pcmcia tree got a conflict in drivers/pcmcia/pxa2xx_viper.c between commit c9b74a4489db7e6fe03fbee11f406b9641baae55 ("[ARM] pxa/zeus: make Viper pcmcia support more generic to support Zeus") from the pxa tree and commit 66024db57d5b9011e274b314affad68f370c0d6f ("PCMCIA: stop duplicating pci_irq in soc_pcmcia_socket") from the pcmcia tree.
I fixed it up (see below) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell [email protected]
diff --cc drivers/pcmcia/pxa2xx_viper.c
index 7a16ae9,27be2e1..0000000
--- a/drivers/pcmcia/pxa2xx_viper.c
+++ b/drivers/pcmcia/pxa2xx_viper.c
@@@ -30,35 -32,23 +30,35 @@@
#include "soc_common.h"
#include "pxa2xx_base.h"
+static struct platform_device *arcom_pcmcia_dev;
+
static struct pcmcia_irqs irqs[] = {
- { 0, gpio_to_irq(VIPER_CF_CD_GPIO), "PCMCIA_CD" }
+ {
+ .sock = 0,
+ .str = "PCMCIA_CD",
+ },
};
+static inline struct arcom_pcmcia_pdata *viper_get_pdata(void)
+{
+ return arcom_pcmcia_dev->dev.platform_data;
+}
+
static int viper_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
{
+ struct arcom_pcmcia_pdata *pdata = viper_get_pdata();
unsigned long flags;
- skt->irq = gpio_to_irq(pdata->rdy_gpio);
- skt->socket.pci_irq = gpio_to_irq(VIPER_CF_RDY_GPIO);
++ skt->socket.pci_irq = gpio_to_irq(pdata->rdy_gpio);
+ irqs[0].irq = gpio_to_irq(pdata->cd_gpio);
- if (gpio_request(VIPER_CF_CD_GPIO, "CF detect"))
+ if (gpio_request(pdata->cd_gpio, "CF detect"))
goto err_request_cd;
- if (gpio_request(VIPER_CF_RDY_GPIO, "CF ready"))
+ if (gpio_request(pdata->rdy_gpio, "CF ready"))
goto err_request_rdy;
- if (gpio_request(VIPER_CF_POWER_GPIO, "CF power"))
+ if (gpio_request(pdata->pwr_gpio, "CF power"))
goto err_request_pwr;
local_irq_save(flags);
Hi Marc,
Could you take a look into this, I wonder you may need to rebase
against Russell's
PCMCIA generalization stuffs. Let me know if you want me to drop this patch.
- eric
On Mon, Nov 30, 2009 at 10:16 AM, Stephen Rothwell <[email protected]> wrote:
> Hi Dominik,
>
> Today's linux-next merge of the pcmcia tree got a conflict in drivers/pcmcia/pxa2xx_viper.c between commit c9b74a4489db7e6fe03fbee11f406b9641baae55 ("[ARM] pxa/zeus: make Viper pcmcia support more generic to support Zeus") from the pxa tree and commit 66024db57d5b9011e274b314affad68f370c0d6f ("PCMCIA: stop duplicating pci_irq in soc_pcmcia_socket") from the pcmcia tree.
>
> I fixed it up (see below) and can carry the fix as necessary.
> --
> Cheers,
> Stephen Rothwell [email protected]
>
> diff --cc drivers/pcmcia/pxa2xx_viper.c
> index 7a16ae9,27be2e1..0000000
> --- a/drivers/pcmcia/pxa2xx_viper.c
> +++ b/drivers/pcmcia/pxa2xx_viper.c
> @@@ -30,35 -32,23 +30,35 @@@
> #include "soc_common.h"
> #include "pxa2xx_base.h"
>
> +static struct platform_device *arcom_pcmcia_dev;
> +
> static struct pcmcia_irqs irqs[] = {
> - { 0, gpio_to_irq(VIPER_CF_CD_GPIO), "PCMCIA_CD" }
> + {
> + .sock = 0,
> + .str = "PCMCIA_CD",
> + },
> };
>
> +static inline struct arcom_pcmcia_pdata *viper_get_pdata(void)
> +{
> + return arcom_pcmcia_dev->dev.platform_data;
> +}
> +
> static int viper_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
> {
> + struct arcom_pcmcia_pdata *pdata = viper_get_pdata();
> unsigned long flags;
>
> - skt->irq = gpio_to_irq(pdata->rdy_gpio);
> - skt->socket.pci_irq = gpio_to_irq(VIPER_CF_RDY_GPIO);
> ++ skt->socket.pci_irq = gpio_to_irq(pdata->rdy_gpio);
> + irqs[0].irq = gpio_to_irq(pdata->cd_gpio);
>
> - if (gpio_request(VIPER_CF_CD_GPIO, "CF detect"))
> + if (gpio_request(pdata->cd_gpio, "CF detect"))
> goto err_request_cd;
>
> - if (gpio_request(VIPER_CF_RDY_GPIO, "CF ready"))
> + if (gpio_request(pdata->rdy_gpio, "CF ready"))
> goto err_request_rdy;
>
> - if (gpio_request(VIPER_CF_POWER_GPIO, "CF power"))
> + if (gpio_request(pdata->pwr_gpio, "CF power"))
> goto err_request_pwr;
>
> local_irq_save(flags);
>
On Mon, 30 Nov 2009 11:17:08 +0800
Eric Miao <[email protected]> wrote:
Hi Eric,
> Could you take a look into this, I wonder you may need to rebase
> against Russell's PCMCIA generalization stuffs. Let me know if you
> want me to drop this patch.
Patch looks good. I'm more worried about creating dependencies between
trees:
- Stephen fix depends on Russell's changes, which are in Dominik's tree,
- My patch depends on Zeus support, which is in your tree,
- Rebasing Zeus PCMCIA support against Dominik's tree is just another
dependency between the two trees.
Should we drop my PCMCIA changes until both Russell's changes and main
Zeus support are merged?
I'm opened to any suggestion.
M.
>
> On Mon, Nov 30, 2009 at 10:16 AM, Stephen Rothwell
> <[email protected]> wrote:
> > Hi Dominik,
> >
> > Today's linux-next merge of the pcmcia tree got a conflict in
> > drivers/pcmcia/pxa2xx_viper.c between commit
> > c9b74a4489db7e6fe03fbee11f406b9641baae55 ("[ARM] pxa/zeus: make
> > Viper pcmcia support more generic to support Zeus") from the pxa
> > tree and commit 66024db57d5b9011e274b314affad68f370c0d6f ("PCMCIA:
> > stop duplicating pci_irq in soc_pcmcia_socket") from the pcmcia
> > tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary.
> > --
> > Cheers,
> > Stephen Rothwell [email protected]
> >
> > diff --cc drivers/pcmcia/pxa2xx_viper.c
> > index 7a16ae9,27be2e1..0000000
> > --- a/drivers/pcmcia/pxa2xx_viper.c
> > +++ b/drivers/pcmcia/pxa2xx_viper.c
> > @@@ -30,35 -32,23 +30,35 @@@
> > #include "soc_common.h"
> > #include "pxa2xx_base.h"
> >
> > +static struct platform_device *arcom_pcmcia_dev;
> > +
> > static struct pcmcia_irqs irqs[] = {
> > - { 0, gpio_to_irq(VIPER_CF_CD_GPIO), "PCMCIA_CD" }
> > + {
> > + .sock = 0,
> > + .str = "PCMCIA_CD",
> > + },
> > };
> >
> > +static inline struct arcom_pcmcia_pdata *viper_get_pdata(void)
> > +{
> > + return arcom_pcmcia_dev->dev.platform_data;
> > +}
> > +
> > static int viper_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
> > {
> > + struct arcom_pcmcia_pdata *pdata = viper_get_pdata();
> > unsigned long flags;
> >
> > - skt->irq = gpio_to_irq(pdata->rdy_gpio);
> > - skt->socket.pci_irq = gpio_to_irq(VIPER_CF_RDY_GPIO);
> > ++ skt->socket.pci_irq = gpio_to_irq(pdata->rdy_gpio);
> > + irqs[0].irq = gpio_to_irq(pdata->cd_gpio);
> >
> > - if (gpio_request(VIPER_CF_CD_GPIO, "CF detect"))
> > + if (gpio_request(pdata->cd_gpio, "CF detect"))
> > goto err_request_cd;
> >
> > - if (gpio_request(VIPER_CF_RDY_GPIO, "CF ready"))
> > + if (gpio_request(pdata->rdy_gpio, "CF ready"))
> > goto err_request_rdy;
> >
> > - if (gpio_request(VIPER_CF_POWER_GPIO, "CF power"))
> > + if (gpio_request(pdata->pwr_gpio, "CF power"))
> > goto err_request_pwr;
> >
> > local_irq_save(flags);
> >
>
--
And if you don't know where you're going, any road will take you
there...
On Mon, Nov 30, 2009 at 4:10 PM, Marc Zyngier <[email protected]> wrote:
> On Mon, 30 Nov 2009 11:17:08 +0800
> Eric Miao <[email protected]> wrote:
>
> Hi Eric,
>
>> Could you take a look into this, I wonder you may need to rebase
>> against Russell's PCMCIA generalization stuffs. Let me know if you
>> want me to drop this patch.
>
> Patch looks good. I'm more worried about creating dependencies between
> trees:
>
> - Stephen fix depends on Russell's changes, which are in Dominik's tree,
> - My patch depends on Zeus support, which is in your tree,
> - Rebasing Zeus PCMCIA support against Dominik's tree is just another
> dependency between the two trees.
>
> Should we drop my PCMCIA changes until both Russell's changes and main
> Zeus support are merged?
>
I'd propose to postpone your patches until dependency is resolved, will
send a second git pull til then. Sounds OK to you?
> I'm opened to any suggestion.
>
> M.
>
>>
>> On Mon, Nov 30, 2009 at 10:16 AM, Stephen Rothwell
>> <[email protected]> wrote:
>> > Hi Dominik,
>> >
>> > Today's linux-next merge of the pcmcia tree got a conflict in
>> > drivers/pcmcia/pxa2xx_viper.c between commit
>> > c9b74a4489db7e6fe03fbee11f406b9641baae55 ("[ARM] pxa/zeus: make
>> > Viper pcmcia support more generic to support Zeus") from the pxa
>> > tree and commit 66024db57d5b9011e274b314affad68f370c0d6f ("PCMCIA:
>> > stop duplicating pci_irq in soc_pcmcia_socket") from the pcmcia
>> > tree.
>> >
>> > I fixed it up (see below) and can carry the fix as necessary.
>> > --
>> > Cheers,
>> > Stephen Rothwell [email protected]
>> >
>> > diff --cc drivers/pcmcia/pxa2xx_viper.c
>> > index 7a16ae9,27be2e1..0000000
>> > --- a/drivers/pcmcia/pxa2xx_viper.c
>> > +++ b/drivers/pcmcia/pxa2xx_viper.c
>> > @@@ -30,35 -32,23 +30,35 @@@
>> > #include "soc_common.h"
>> > #include "pxa2xx_base.h"
>> >
>> > +static struct platform_device *arcom_pcmcia_dev;
>> > +
>> > static struct pcmcia_irqs irqs[] = {
>> > - { 0, gpio_to_irq(VIPER_CF_CD_GPIO), "PCMCIA_CD" }
>> > + {
>> > + .sock = 0,
>> > + .str = "PCMCIA_CD",
>> > + },
>> > };
>> >
>> > +static inline struct arcom_pcmcia_pdata *viper_get_pdata(void)
>> > +{
>> > + return arcom_pcmcia_dev->dev.platform_data;
>> > +}
>> > +
>> > static int viper_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
>> > {
>> > + struct arcom_pcmcia_pdata *pdata = viper_get_pdata();
>> > unsigned long flags;
>> >
>> > - skt->irq = gpio_to_irq(pdata->rdy_gpio);
>> > - skt->socket.pci_irq = gpio_to_irq(VIPER_CF_RDY_GPIO);
>> > ++ skt->socket.pci_irq = gpio_to_irq(pdata->rdy_gpio);
>> > + irqs[0].irq = gpio_to_irq(pdata->cd_gpio);
>> >
>> > - if (gpio_request(VIPER_CF_CD_GPIO, "CF detect"))
>> > + if (gpio_request(pdata->cd_gpio, "CF detect"))
>> > goto err_request_cd;
>> >
>> > - if (gpio_request(VIPER_CF_RDY_GPIO, "CF ready"))
>> > + if (gpio_request(pdata->rdy_gpio, "CF ready"))
>> > goto err_request_rdy;
>> >
>> > - if (gpio_request(VIPER_CF_POWER_GPIO, "CF power"))
>> > + if (gpio_request(pdata->pwr_gpio, "CF power"))
>> > goto err_request_pwr;
>> >
>> > local_irq_save(flags);
>> >
>>
>
>
> --
> And if you don't know where you're going, any road will take you
> there...
>
On Mon, 30 Nov 2009 16:16:38 +0800
Eric Miao <[email protected]> wrote:
> On Mon, Nov 30, 2009 at 4:10 PM, Marc Zyngier <[email protected]>
> wrote:
> > On Mon, 30 Nov 2009 11:17:08 +0800
> > Eric Miao <[email protected]> wrote:
> >
> > Hi Eric,
> >
> >> Could you take a look into this, I wonder you may need to rebase
> >> against Russell's PCMCIA generalization stuffs. Let me know if you
> >> want me to drop this patch.
> >
> > Patch looks good. I'm more worried about creating dependencies
> > between trees:
> >
> > - Stephen fix depends on Russell's changes, which are in Dominik's
> > tree,
> > - My patch depends on Zeus support, which is in your tree,
> > - Rebasing Zeus PCMCIA support against Dominik's tree is just
> > another dependency between the two trees.
> >
> > Should we drop my PCMCIA changes until both Russell's changes and
> > main Zeus support are merged?
> >
>
> I'd propose to postpone your patches until dependency is resolved,
> will send a second git pull til then. Sounds OK to you?
Sounds good. I'll post rebased patches as soon as PCMCIA hits Linus'
tree.
Thanks,
M.
--
Fast. Cheap. Reliable. Pick two.
On Mon, Nov 30, 2009 at 10:24:44AM +0100, Marc Zyngier wrote:
> Sounds good. I'll post rebased patches as soon as PCMCIA hits Linus'
> tree.
What would be a better idea is to rework the driver once my changes have
hit. If you need to pass additional data around, you can do it like this:
struct my_socket {
struct soc_pcmcia_socket skt;
...
};
#define to_my_socket(s) container_of(s, struct my_socket, skt)
static struct pcmcia_low_level my_ops = {
};
static int my_probe(struct platform_device *dev)
{
struct my_socket *skts;
pxa2xx_drv_pcmcia_ops(&my_ops);
skts = kzalloc(sizeof(*skts) * nr, GFP_KERNEL);
if (!skts)
...
for (i = 0; i < nr; i++) {
skts[i].skt.nr = i;
skts[i].skt.ops = &my_ops;
skts[i].skt.socket.owner = THIS_MODULE;
skts[i].skt.socket.parent = &dev->dev;
skts[i].skt.socket.pci_irq = the_socket_irq;
ret = pxa2xx_drv_pcmcia_add_one(&skts[i].skt);
if (ret)
goto free_registered_sockets_and_return_ret;
}
}
On Mon, 30 Nov 2009 09:48:13 +0000
Russell King - ARM Linux <[email protected]> wrote:
Hi Russell,
> What would be a better idea is to rework the driver once my changes
> have hit. If you need to pass additional data around, you can do it
> like this:
Now that your changes are in Linus' tree, I started looking at
restructuring my code. I sense a small problem though:
[...]
> for (i = 0; i < nr; i++) {
> skts[i].skt.nr = i;
> skts[i].skt.ops = &my_ops;
> skts[i].skt.socket.owner = THIS_MODULE;
> skts[i].skt.socket.parent = &dev->dev;
> skts[i].skt.socket.pci_irq = the_socket_irq;
> ret = pxa2xx_drv_pcmcia_add_one(&skts[i].skt);
Unless I'm mistaken, it seems that pxa2xx_drv_pcmcia_add_one() doesn't
call pxa2xx_configure_sockets(), leaving the sockets disabled.
There is several ways to solve this one:
- Add the call to pxa2xx_configure_sockets() to _add_one()
- Do the MECR access in the pxa2xx_viper driver,
- export pxa2xx_configure_sockets() and do the call locally.
What would be your preferred solution?
M.
--
I'm the slime oozin' out from your TV set...