>>>>> "Andreas" == Andreas Larsson <andr...@gaisler.com> writes:
Andreas> The registers in the GRLIB port of the controller are 32-bit
Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
Andreas> are merged into one register. The subsequent registers have
Andreas> their offset decreased accordingly. Hence the register access
Andreas> needs to be handled in a non-standard manner using custom
Andreas> getreg and setreg functions.
Andreas> The single oc_getreg and one oc_setreg functions, that
Andreas> branches and call different ioread and iowrite functions, are
Andreas> replaced by specific functions that uses the appropriate
Andreas> ioread and iowrite functions and function pointers are
Andreas> initiated and used to call the approproate functions.
Andreas> A type is added as the data of the of match table entries. A
Andreas> new entry with a different compatible string is added to the
Andreas> table. The type of that entry triggers usage of the custom
Andreas> grlib functions by setting the new getreg and setreg function
Andreas> pointers to these functions.
Sorry to keep on requesting changes, but see below for a bit more:
Andreas> static void ocores_process(struct ocores_i2c *i2c)
Andreas> {
Andreas> struct i2c_msg *msg = i2c->msg;
Andreas> - u8 stat = oc_getreg(i2c, OCI2C_STATUS);
Andreas> + u8 stat = i2c->getreg(i2c, OCI2C_STATUS);
The patch would have been quite a bit smaller/easier to read if you had
kept oc_getreg / oc_setreg as wrappers.
Andreas> #ifdef CONFIG_OF
Andreas> static int ocores_i2c_of_probe(struct platform_device *pdev,
Andreas> struct ocores_i2c *i2c)
Andreas> {
Andreas> struct device_node *np = pdev->dev.of_node;
Andreas> + const struct of_device_id *match;
Andreas> u32 val;
Andreas> if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
Andreas> @@ -257,6 +324,14 @@ static int ocores_i2c_of_probe(struct
platform_device *pdev,
Andreas> of_property_read_u32(pdev->dev.of_node, "reg-io-width",
Andreas> &i2c->reg_io_width);
Andreas> +
Andreas> + match = of_match_node(ocores_i2c_match, pdev->dev.of_node);
Andreas> + if (match && (int)match->data == TYPE_GRLIB) {
Andreas> + dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
Andreas> + i2c->setreg = oc_setreg_grlib;
Andreas> + i2c->getreg = oc_getreg_grlib;
Andreas> + }
Andreas> +
Andreas> return 0;
Andreas> }
Andreas> #else
Andreas> @@ -311,6 +386,23 @@ static int __devinit ocores_i2c_probe(struct
platform_device *pdev)
Andreas> if (i2c->reg_io_width == 0)
i2c-> reg_io_width = 1; /* Set to default value */
Andreas> + if (!i2c->getreg) {
Andreas> + if (i2c->reg_io_width == 4)
Andreas> + i2c->getreg = oc_getreg_32;
Andreas> + else if (i2c->reg_io_width == 2)
Andreas> + i2c->getreg = oc_getreg_16;
Andreas> + else
Andreas> + i2c->getreg = oc_getreg_8;
Andreas> + }
Andreas> + if (!i2c->setreg) {
Andreas> + if (i2c->reg_io_width == 4)
Andreas> + i2c->setreg = oc_setreg_32;
Andreas> + else if (i2c->reg_io_width == 2)
Andreas> + i2c->setreg = oc_setreg_16;
Andreas> + else
Andreas> + i2c->setreg = oc_setreg_8;
Andreas> + }
Andreas> +
These are always set together (could even move to a single ops
structure), so it would be shorter to assign them at the same time:
if (!i2c->setreg || !i2c->getreg) {
switch (i2c->reg_io_width) {
case 1:
i2c->setreg = oc_setreg_8;
i2c->getreg = oc_getreg_8;
break;
case 2:
i2c->setreg = oc_setreg_16;
...
default:
dev_err(&pdev->dev "Unsupported I/O width (%d)\n", i2c->reg_io_width);
return -EINVAL;
}
}
--
Bye, Peter Korsgaard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
