Dear Jakub Kicinski
On Mon, 2023-03-13 at 16:08 -0700, Jakub Kicinski wrote:
> On Wed, 8 Mar 2023 16:19:35 +0800 haozhe.chang@xxxxxxxxxxxx wrote:
> > /**
> > * wwan_create_port - Add a new WWAN port
> > * @parent: Device to use as parent and shared by all WWAN ports
> > * @type: WWAN port type
> > * @ops: WWAN port operations
> > + * @frag_len: WWAN port TX fragments length, if WWAN_NO_FRAGMENT
> > is set,
> > + * the WWAN core don't fragment control packages.
> > + * @headroom_len: WWAN port TX fragments reserved headroom length,
> > if WWAN_NO_HEADROOM
> > + * is set, the WWAN core don't reserve headroom in
> > control packages.
> > * @drvdata: Pointer to caller driver data
> > *
> > * Allocate and register a new WWAN port. The port will be
> > automatically exposed
> > @@ -86,6 +100,8 @@ struct wwan_port_ops {
> > struct wwan_port *wwan_create_port(struct device *parent,
> > enum wwan_port_type type,
> > const struct wwan_port_ops *ops,
> > + size_t frag_len,
> > + unsigned int headroom_len,
> > void *drvdata);
> >
>
> Too many arguments, and poor extensibility.
> Please wrap the new params into a capability struct:
>
> struct wwan_port_caps {
> unsigned int frag_len;
> unsigned int headroom_len;
> };
>
> pass a pointer to this kind of structure in.
>
> Next time someone needs to add a quirk they can just add a field and
> won't need to change all the drivers.
Thanks for your suggestion. But the same suggestion has been discussed
in previous patch v2:
https://patchwork.kernel.org/project/netdevbpf/patch/20221108105352.89801-1-haozhe.chang@xxxxxxxxxxxx/
The opinion from Loic: "I think 6 is still fine, if we need more fields
in the future we can always have a port_config struct as a parameter
instead."
BR
