Re: [PATCH v3] bcma: switch GPIO portions to use GPIOLIB_IRQCHIP

[Linus Walleij]

On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens  wrote:
> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
>> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>
> Yes, you are probably right.
>
>> On 14 August 2015 at 00:21, Hauke Mehrtens  wrote:
>>> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>> chip->set   = bcma_gpio_set_value;
>>> chip->direction_input   = bcma_gpio_direction_input;
>>> chip->direction_output  = bcma_gpio_direction_output;
>>> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
>>> -   chip->to_irq= bcma_gpio_to_irq;
>>> -#endif
>>> +   chip->owner = THIS_MODULE;
>>> +   chip->dev   = bcma_bus_get_host_dev(bus);
>>
>> This assigns >host_pdev->dev which is NULL.
>
> hmm, how do we fix this, as long as bcma does not have a device on mips
> this is a problem. Should we create a new device for mips?

Yes I think MIPS should just create a host device. That
makes things simple and nice. Is it complex to do so?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] bcma: switch GPIO portions to use GPIOLIB_IRQCHIP

[Hauke Mehrtens]

On 12/22/2015 10:01 AM, Linus Walleij wrote:
> On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens  wrote:
>> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
>>> I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>>
>> Yes, you are probably right.
>>
>>> On 14 August 2015 at 00:21, Hauke Mehrtens  wrote:
 @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
 chip->set   = bcma_gpio_set_value;
 chip->direction_input   = bcma_gpio_direction_input;
 chip->direction_output  = bcma_gpio_direction_output;
 -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
 -   chip->to_irq= bcma_gpio_to_irq;
 -#endif
 +   chip->owner = THIS_MODULE;
 +   chip->dev   = bcma_bus_get_host_dev(bus);
>>>
>>> This assigns >host_pdev->dev which is NULL.
>>
>> hmm, how do we fix this, as long as bcma does not have a device on mips
>> this is a problem. Should we create a new device for mips?
> 
> Yes I think MIPS should just create a host device. That
> makes things simple and nice. Is it complex to do so?
> 
> Yours,
> Linus Walleij
> 
Currently the documentation says this for the parent member on struct
gpio_chip:

 * @parent: optional parent device providing the GPIOs

I assume the optional is wrong here.

Hauke
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] bcma: switch GPIO portions to use GPIOLIB_IRQCHIP

[Linus Walleij]

On Tue, Dec 22, 2015 at 1:09 PM, Hauke Mehrtens  wrote:
> On 12/22/2015 10:01 AM, Linus Walleij wrote:
>> On Fri, Dec 18, 2015 at 7:01 PM, Hauke Mehrtens  wrote:
>>> On 12/18/2015 07:27 AM, Rafał Miłecki wrote:
 I'm afraid it wasn't tested on BCM47XX (MIPS) :(
>>>
>>> Yes, you are probably right.
>>>
 On 14 August 2015 at 00:21, Hauke Mehrtens  wrote:
> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> chip->set   = bcma_gpio_set_value;
> chip->direction_input   = bcma_gpio_direction_input;
> chip->direction_output  = bcma_gpio_direction_output;
> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
> -   chip->to_irq= bcma_gpio_to_irq;
> -#endif
> +   chip->owner = THIS_MODULE;
> +   chip->dev   = bcma_bus_get_host_dev(bus);

 This assigns >host_pdev->dev which is NULL.
>>>
>>> hmm, how do we fix this, as long as bcma does not have a device on mips
>>> this is a problem. Should we create a new device for mips?
>>
>> Yes I think MIPS should just create a host device. That
>> makes things simple and nice. Is it complex to do so?
>>
>> Yours,
>> Linus Walleij
>>
> Currently the documentation says this for the parent member on struct
> gpio_chip:
>
>  * @parent: optional parent device providing the GPIOs
>
> I assume the optional is wrong here.

For just gpiochip it is optional. But if you read on:

GPIO drivers providing IRQs
---
(...)
To use the helpers please keep the following in mind:

- Make sure to assign all relevant members of the struct gpio_chip so that
  the irqchip can initialize. E.g. .dev and .can_sleep shall be set up
  properly.
(...)

Well it's complex. But when adding the gpio character device I
am working with, I will introduce a struct device to the gpio_chip
(or similar sibling gpio_device) which we can then refer, so that
this actually will no longer be a requirement.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] bcma: switch GPIO portions to use GPIOLIB_IRQCHIP

[Rafał Miłecki]

I'm afraid it wasn't tested on BCM47XX (MIPS) :(

On 14 August 2015 at 00:21, Hauke Mehrtens  wrote:
> @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> chip->set   = bcma_gpio_set_value;
> chip->direction_input   = bcma_gpio_direction_input;
> chip->direction_output  = bcma_gpio_direction_output;
> -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X)
> -   chip->to_irq= bcma_gpio_to_irq;
> -#endif
> +   chip->owner = THIS_MODULE;
> +   chip->dev   = bcma_bus_get_host_dev(bus);

This assigns >host_pdev->dev which is NULL.


> @@ -248,13 +216,13 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> else
> chip->base  = -1;
>
> -   err = bcma_gpio_irq_domain_init(cc);
> +   err = gpiochip_add(chip);
> if (err)
> return err;
>
> -   err = gpiochip_add(chip);
> +   err = bcma_gpio_irq_init(cc);

This results in:
[0.157054] missing gpiochip .dev parent pointer
(coming from gpiochip_irqchip_add) and
[0.157287] bcma: bus0: Error registering GPIO driver: -22
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html