On Tue, Oct 24, 2023 at 07:29:11PM +0800, bingbu.cao@xxxxxxxxx wrote:
> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>
> Even the IPU input system and processing system are in a single PCI
> device, each system has its own power sequence, the processing system
> power up depends on the input system power up.
>
> Besides, input system and processing system have their own MMU
> hardware for IPU DMA address mapping.
>
> Register the IS/PS devices on auxiliary bus and attach power domain
> to implement the power sequence dependency.
...
Seems again poor / random list of header inclusions...
Please, go through your files and use IWYU (Include What You Use) principle
in them.
> +#include <linux/pci.h>
auxiliary_bus.h, err.h, list.h, mutex.h, dev_printk.h, slab.h, dma-mapping.h are missing.
You are lucky it does compile.
...
> +#ifndef IPU6_BUS_H
> +#define IPU6_BUS_H
> +
> +#include <linux/auxiliary_bus.h>
...Especially for headers which will affect the compilation time.
> +#include <linux/pci.h>
This is not used.
...
> + struct list_head list;
+ types.h
> + struct sg_table fw_sgt;
+ scatterlist.h
> + dma_addr_t pkg_dir_dma_addr;
types.h
...
> +struct ipu6_auxdrv_data {
> + irqreturn_t (*isr)(struct ipu6_bus_device *adev);
> + irqreturn_t (*isr_threaded)(struct ipu6_bus_device *adev);
irqreturn.h
> + bool wake_isr_thread;
> +};
...
> +#define to_ipu6_bus_device(_dev) container_of(to_auxiliary_dev(_dev), \
> + struct ipu6_bus_device, auxdev)
> +#define auxdev_to_adev(_auxdev) container_of(_auxdev, \
> + struct ipu6_bus_device, auxdev)
container_of.h
Also, can you reformat to be more readable like
#define auxdev_to_adev(_auxdev) \
container_of(_auxdev, struct ipu6_bus_device, auxdev)
> +#define ipu6_bus_get_drvdata(adev) dev_get_drvdata(&(adev)->auxdev.dev)
device.h
...
> + ret = dma_set_max_seg_size(&pdev->dev, UINT_MAX);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to set max_seg_size\n");
With the help of
struct device *dev = &pdev->dev;
this and other lines become neater
ret = dma_set_max_seg_size(dev, _DMA_SEGMENT_SIZE); // as I commented earlier about
if (ret)
return dev_err_probe(dev, ret, "Failed to set max_seg_size\n");
--
With Best Regards,
Andy Shevchenko
