Re: [PATCH net-next v3] net: txgbe: Add build support for txgbe

[Leon Romanovsky <leon@kernel.org>]

On Fri, May 27, 2022 at 02:31:57PM +0800, Jiawen Wu wrote:
> Add doc build infrastructure for txgbe driver.
> Initialize PCI memory space for WangXun 10 Gigabit Ethernet devices.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  .../device_drivers/ethernet/index.rst         |   1 +
>  .../device_drivers/ethernet/wangxun/txgbe.rst |  20 ++
>  MAINTAINERS                                   |   7 +
>  drivers/net/ethernet/Kconfig                  |   1 +
>  drivers/net/ethernet/Makefile                 |   1 +
>  drivers/net/ethernet/wangxun/Kconfig          |  32 +++
>  drivers/net/ethernet/wangxun/Makefile         |   6 +
>  drivers/net/ethernet/wangxun/txgbe/Makefile   |   9 +
>  drivers/net/ethernet/wangxun/txgbe/txgbe.h    |  27 ++
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 241 ++++++++++++++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  65 +++++
>  11 files changed, 410 insertions(+)
>  create mode 100644 Documentation/networking/device_drivers/ethernet/wangxun/txgbe.rst
>  create mode 100644 drivers/net/ethernet/wangxun/Kconfig
>  create mode 100644 drivers/net/ethernet/wangxun/Makefile
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/Makefile
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe.h
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
>  create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_type.h

<...>

> +/**
> + * txgbe_sw_init - Initialize general software structures (struct txgbe_adapter)
> + * @adapter: board private structure to initialize
> + *
> + * txgbe_sw_init initializes the Adapter private data structure.
> + * Fields are initialized based on PCI device information and
> + * OS network device settings (MTU size).
> + **/
> +static int txgbe_sw_init(struct txgbe_adapter *adapter)
> +{
> +	struct txgbe_hw *hw = &adapter->hw;
> +	struct pci_dev *pdev = adapter->pdev;
> +
> +	/* PCI config space info */
> +	hw->vendor_id = pdev->vendor;
> +	hw->device_id = pdev->device;
> +	hw->revision_id = pdev->revision;
> +	hw->subsystem_vendor_id = pdev->subsystem_vendor;
> +	hw->subsystem_device_id = pdev->subsystem_device;
> +
> +	return 0;
> +}

You are setting this data, but not using it. I'm not sure that to cache
this data is even correct thing to do in a first place.

> +

<...>

> +	pci_set_master(pdev);
> +	/* errata 16 */
> +	pcie_capability_clear_and_set_word(pdev, PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_READRQ,
> +					   0x1000);

Why do you need this in probe function and not as PCI quirk?

Thanks