On Mon, Jul 14, 2025 at 12:35:14PM +0200, Quentin Schulz wrote: > Hi Tom, > > On 7/2/25 3:05 AM, Tom Rini wrote: > > For 32/64bit correctness, we need to use ulong and not u32 for casting > > for addresses. > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > --- > > Cc: Lukasz Majewski <lu...@denx.de> > > Cc: Sean Anderson <sean...@gmail.com> > > --- > > drivers/clk/clk-cdce9xx.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clk/clk-cdce9xx.c b/drivers/clk/clk-cdce9xx.c > > index e5f74e714d54..afb997c06be5 100644 > > --- a/drivers/clk/clk-cdce9xx.c > > +++ b/drivers/clk/clk-cdce9xx.c > > @@ -103,7 +103,7 @@ static int cdce9xx_clk_probe(struct udevice *dev) > > u32 val; > > struct clk clk; > > - val = (u32)dev_read_addr_ptr(dev); > > + val = (ulong)dev_read_addr_ptr(dev); > > The output would be stored in a u32 anyway so not sure this actually helps > (see type of val in the git context above).
Yeah. It's funny. The other example of a driver doing these games is drivers/clk/clk_versaclock.c which uses u64 since it's a 64bit system I believe. > > ret = i2c_get_chip(dev->parent, val, 1, &data->i2c); > > if (ret) { > > @@ -226,10 +226,10 @@ static ulong cdce9xx_clk_set_rate(struct clk *clk, > > ulong rate) > > } > > static const struct udevice_id cdce9xx_clk_of_match[] = { > > - { .compatible = "ti,cdce913", .data = (u32)&cdce913_chip_info }, > > - { .compatible = "ti,cdce925", .data = (u32)&cdce925_chip_info }, > > - { .compatible = "ti,cdce937", .data = (u32)&cdce937_chip_info }, > > - { .compatible = "ti,cdce949", .data = (u32)&cdce949_chip_info }, > > + { .compatible = "ti,cdce913", .data = (ulong)&cdce913_chip_info }, > > + { .compatible = "ti,cdce925", .data = (ulong)&cdce925_chip_info }, > > + { .compatible = "ti,cdce937", .data = (ulong)&cdce937_chip_info }, > > + { .compatible = "ti,cdce949", .data = (ulong)&cdce949_chip_info }, > > Just get rid of the cast I guess? udevice_id.data being a ulong already the > compiler should perform the cast in any case and this improves readability? Without some cast we get: error: initialization of ‘long unsigned int’ from ‘const struct cdce9xx_chip_info *’ makes integer from pointer without a cast [-Werror=int-conversion] That said, I'm just going to drop this patch and make the driver depend on ARCH_OMAP2PLUS. My gut feeling is this is another one of the cases where we run in to problems because we don't use phys_addr_t for physical addresses consistently. -- Tom
signature.asc
Description: PGP signature