Re: [PATCH v2] pata_parport: add driver (PARIDE replacement)

Ondrej Zary
2023-01-23T19:13:57Z

On Monday 23 January 2023 02:03:16 Damien Le Moal wrote:

[...]

> You are missing:
>
> #ifndef LINUX_PATA_PARPORT_H
> #define LINUX_PATA_PARPORT_H
>
> > +#include <linux/libata.h>
> > +
> > +#define PI_PCD 1 /* dummy for paride protocol modules */
> > +
> > +struct pi_adapter {
> > + struct device dev;
> > + struct pi_protocol *proto; /* adapter protocol */
> > + int port; /* base address of parallel port */
> > + int mode; /* transfer mode in use */
> > + int delay; /* adapter delay setting */
> > + int devtype; /* dummy for paride protocol modules */
> > + char *device; /* dummy for paride protocol modules */
> > + int unit; /* unit number for chained adapters */
> > + int saved_r0; /* saved port state */
> > + int saved_r2; /* saved port state */
> > + unsigned long private; /* for protocol module */
> > + struct pardevice *pardev; /* pointer to pardevice */
> > +};
> > +
> > +typedef struct pi_adapter PIA; /* for paride protocol modules */
> > +
> > +/* registers are addressed as (cont,regr)
> > + * cont: 0 for command register file, 1 for control register(s)
> > + * regr: 0-7 for register number.
> > + */
> > +
> > +/* macros and functions exported to the protocol modules */
> > +#define delay_p (pi->delay ? udelay(pi->delay) : (void)0)
> > +#define out_p(offs, byte) do { outb(byte, pi->port + offs); delay_p; } while (0)
> > +#define in_p(offs) (delay_p, inb(pi->port + offs))
>
> It would be way nicer to have these as inline functions.

This should go away (as well as the code below) so I'm not touching it now.

> > +
> > +#define w0(byte) out_p(0, byte)
> > +#define r0() in_p(0)
> > +#define w1(byte) out_p(1, byte)
> > +#define r1() in_p(1)
> > +#define w2(byte) out_p(2, byte)
> > +#define r2() in_p(2)
> > +#define w3(byte) out_p(3, byte)
> > +#define w4(byte) out_p(4, byte)
> > +#define r4() in_p(4)
> > +#define w4w(data) do { outw(data, pi->port + 4); delay_p; } while (0)
> > +#define w4l(data) do { outl(data, pi->port + 4); delay_p; } while (0)
> > +#define r4w() (delay_p, inw(pi->port + 4))
> > +#define r4l() (delay_p, inl(pi->port + 4))
> > +
> > +static inline u16 pi_swab16(char *b, int k)
> > +{
> > + union { u16 u; char t[2]; } r;
> > +
> > + r.t[0] = b[2 * k + 1]; r.t[1] = b[2 * k];
> > + return r.u;
> > +}
> > +
> > +static inline u32 pi_swab32(char *b, int k)
> > +{
> > + union { u32 u; char f[4]; } r;
> > +
> > + r.f[0] = b[4 * k + 1]; r.f[1] = b[4 * k];
> > + r.f[2] = b[4 * k + 3]; r.f[3] = b[4 * k + 2];
> > + return r.u;
> > +}

--
Ondrej Zary