Re: [PATCH V3 5/9] brcmfmac: pcie: store private pointer to struct brcmf_pub

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

On 2-2-2017 22:33, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Getting this pointer in PCIe code is not trivial and requires using
> dev_get_drvdata helper which adds extra line of code. Having access to
> this struct is useful for using generic stuff and e.g. improving logging
> messages.

It is actually by design that getting the pointer is not trivial. We
have made an effort to use struct device pointer as handle between
common and bus-specific code. So I have some reservations about the
approach in this patch series. I had a few days off so I want to look at
it in more detail tomorrow and probably give it a spin.

Regards,
Arend

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 6fae4cf3f6ab..8a3c6e2e4b38 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -265,6 +265,7 @@ struct brcmf_pciedev_info {
>  	void (*write_ptr)(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
>  			  u16 value);
>  	struct brcmf_mp_device *settings;
> +	struct brcmf_pub *pub;
>  };
>  
>  struct brcmf_pcie_ringbuf {
> @@ -1564,14 +1565,18 @@ static void brcmf_pcie_release_resource(struct brcmf_pciedev_info *devinfo)
>  
>  static int brcmf_pcie_attach_bus(struct brcmf_pciedev_info *devinfo)
>  {
> +	struct device *dev = &devinfo->pdev->dev;
> +	struct brcmf_bus *bus = dev_get_drvdata(dev);
>  	int ret;
>  
>  	/* Attach to the common driver interface */
> -	ret = brcmf_attach(&devinfo->pdev->dev, devinfo->settings);
> +	ret = brcmf_attach(dev, devinfo->settings);
>  	if (ret) {
>  		brcmf_err("brcmf_attach failed\n");
>  	} else {
> -		ret = brcmf_bus_started(&devinfo->pdev->dev);
> +		devinfo->pub = bus->drvr;
> +
> +		ret = brcmf_bus_started(dev);
>  		if (ret)
>  			brcmf_err("dongle is not responding\n");
>  	}
>