Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

[Andrew Lunn]

> diff --git a/drivers/misc/tc956x_pci.c b/drivers/misc/tc956x_pci.c

> +static inline void chip_reset_assert(const struct tc956x_chip *chip,
> +				     enum reset_id id)
> +{
> +	tc956x_reset_clock_set(chip, true, true, true, (u8)id);
> +}

This is in drivers/misc, where the rules might be different. But in
netdev, we don't like inline functions in .c files. It is better to
let the compiler decide.

> +static void chip_init_state(struct tc956x_chip *chip)
> +{
> +	/* The only IP block we currently use is MSIGEN */
> +	chip_reset_assert(chip, RESET_MCU);
> +	chip_reset_assert(chip, RESET_MCU1);
> +	chip_reset_assert(chip, RESET_INTC);
> +	chip_reset_assert(chip, RESET_UART0);
> +	chip_clock_disable(chip, CLOCK_MCU);
> +	chip_clock_disable(chip, CLOCK_SRAM);
> +	chip_clock_disable(chip, CLOCK_PLL);
> +	chip_clock_disable(chip, CLOCK_SGMII);

With my networking hat on, this one standard out.

> +	chip_clock_disable(chip, CLOCK_REFCLK);

The name REFCLK is sometimes used as for the clock signals for RGMII?

> +static int
> +tc956x_function_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc956x_chip *chip;
> +	unsigned int msigen_irq;
> +	int ret;
> +
> +	/* Despite being a PCI device, we require devicetree */
> +	if (!dev->of_node)
> +		return -EINVAL;

Might be worth a dev_err(), since it is unusual.

	Andrew