| Message ID | 1415360125-8655-1-git-send-email-andy.yan@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Freitag, den 07.11.2014, 19:35 +0800 schrieb Andy Yan: > On rockchip rk3288, only word(32-bit) accesses are > permitted for hdmi registers. Byte width access (writeb, > readb) generates an imprecise external abort. > > Signed-off-by: Andy Yan <andy.yan@rock-chips.com> > --- > drivers/gpu/drm/bridge/dw_hdmi.c | 49 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 46 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c > index df76a8c..9867642 100644 > --- a/drivers/gpu/drm/bridge/dw_hdmi.c > +++ b/drivers/gpu/drm/bridge/dw_hdmi.c > @@ -126,19 +126,42 @@ struct dw_hdmi { [...] > + u32 val; > + > + if (!of_property_read_u32(np, "reg-io-width", &val)) { > + switch (val) { > + case 4: > + hdmi->write = dw_hdmi_writel; > + hdmi->read = dw_hdmi_readl; > + hdmi->reg_shift = 2; > + break; > + default: > + hdmi->write = dw_hdmi_writeb; > + hdmi->read = dw_hdmi_readb; > + hdmi->reg_shift = 0; > + break; > + } > + } else { > + hdmi->write = dw_hdmi_writeb; > + hdmi->read = dw_hdmi_readb; > + hdmi->reg_shift = 0; > + } > > ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); > if (ddc_node) { This should throw an error if the property value in devicetree is not recognized. This could be simplified like this: u32 val = 1; // this won't touch val if it can't find the property of_property_read_u32(np, "reg-io-width", &val) switch (val) { case 4: hdmi->write = dw_hdmi_writel; hdmi->read = dw_hdmi_readl; hdmi->reg_shift = 2; break; case 1: hdmi->write = dw_hdmi_writeb; hdmi->read = dw_hdmi_readb; hdmi->reg_shift = 0; break; default: dev_err(dev, "unrecognized value for reg-io-width"); // error handling } Also the DT binding doc for this property is missing. Regards, Lucas
On 2014?11?07? 19:45, Lucas Stach wrote: > Am Freitag, den 07.11.2014, 19:35 +0800 schrieb Andy Yan: >> On rockchip rk3288, only word(32-bit) accesses are >> permitted for hdmi registers. Byte width access (writeb, >> readb) generates an imprecise external abort. >> >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >> --- >> drivers/gpu/drm/bridge/dw_hdmi.c | 49 +++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 46 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c >> index df76a8c..9867642 100644 >> --- a/drivers/gpu/drm/bridge/dw_hdmi.c >> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c >> @@ -126,19 +126,42 @@ struct dw_hdmi { > [...] >> + u32 val; >> + >> + if (!of_property_read_u32(np, "reg-io-width", &val)) { >> + switch (val) { >> + case 4: >> + hdmi->write = dw_hdmi_writel; >> + hdmi->read = dw_hdmi_readl; >> + hdmi->reg_shift = 2; >> + break; >> + default: >> + hdmi->write = dw_hdmi_writeb; >> + hdmi->read = dw_hdmi_readb; >> + hdmi->reg_shift = 0; >> + break; >> + } >> + } else { >> + hdmi->write = dw_hdmi_writeb; >> + hdmi->read = dw_hdmi_readb; >> + hdmi->reg_shift = 0; >> + } >> >> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); >> if (ddc_node) { > This should throw an error if the property value in devicetree is not > recognized. This could be simplified like this: > > u32 val = 1; > > // this won't touch val if it can't find the property > of_property_read_u32(np, "reg-io-width", &val) > > > switch (val) { > case 4: > hdmi->write = dw_hdmi_writel; > hdmi->read = dw_hdmi_readl; > hdmi->reg_shift = 2; > break; > case 1: > hdmi->write = dw_hdmi_writeb; > hdmi->read = dw_hdmi_readb; > hdmi->reg_shift = 0; > break; > default: > dev_err(dev, "unrecognized value for reg-io-width"); > // error handling > } > > Also the DT binding doc for this property is missing. > > Regards, > Lucas thanks for your suggestion, I will take it. this properity is optional, I will add it to DT binding doc
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index df76a8c..9867642 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -126,19 +126,42 @@ struct dw_hdmi { unsigned int sample_rate; int ratio; + void (*write)(u8 val, void __iomem *addr); + u8 (*read)(void __iomem *addr); + int reg_shift; }; +static void dw_hdmi_writel(u8 val, void __iomem *addr) +{ + writel(val, addr); +} + +static u8 dw_hdmi_readl(void __iomem *addr) +{ + return readl(addr); +} + +static void dw_hdmi_writeb(u8 val, void __iomem *addr) +{ + writeb(val, addr); +} + +static u8 dw_hdmi_readb(void __iomem *addr) +{ + return readb(addr); +} + static inline void hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset) { - writeb(val, hdmi->regs + offset); + hdmi->write(val, hdmi->regs + (offset << hdmi->reg_shift)); } static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset) { - return readb(hdmi->regs + offset); + return hdmi->read(hdmi->regs + (offset << hdmi->reg_shift)); } -static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg) +static void hdmi_modb(struct dw_hdmi *hdmi, u32 data, u32 mask, unsigned reg) { u8 val = hdmi_readb(hdmi, reg) & ~mask; @@ -1499,6 +1522,26 @@ static int dw_hdmi_bind(struct device *dev, struct device *master, void *data) struct device_node *ddc_node; struct resource *iores; int ret, irq; + u32 val; + + if (!of_property_read_u32(np, "reg-io-width", &val)) { + switch (val) { + case 4: + hdmi->write = dw_hdmi_writel; + hdmi->read = dw_hdmi_readl; + hdmi->reg_shift = 2; + break; + default: + hdmi->write = dw_hdmi_writeb; + hdmi->read = dw_hdmi_readb; + hdmi->reg_shift = 0; + break; + } + } else { + hdmi->write = dw_hdmi_writeb; + hdmi->read = dw_hdmi_readb; + hdmi->reg_shift = 0; + } ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); if (ddc_node) {
On rockchip rk3288, only word(32-bit) accesses are permitted for hdmi registers. Byte width access (writeb, readb) generates an imprecise external abort. Signed-off-by: Andy Yan <andy.yan@rock-chips.com> --- drivers/gpu/drm/bridge/dw_hdmi.c | 49 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-)