[PATCH v6 19/20] m68k/mac: Switch to use %ptR
Use %ptR instead of open coded variant to print content of
struct rtc_time in human readable format.
Cc: Geert Uytterhoeven
Cc: linux-m68k
Signed-off-by: Andy Shevchenko
Reviewed-by: Geert Uytterhoeven
Acked-by: Geert Uytterhoeven
---
arch/m68k/mac/misc.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/m68k/mac/misc.c b/arch/m68k/mac/misc.c
index ebb3b6d169ea..71c4735a31ee 100644
--- a/arch/m68k/mac/misc.c
+++ b/arch/m68k/mac/misc.c
@@ -605,13 +605,9 @@ int mac_hwclk(int op, struct rtc_time *t)
unmktime(now, 0,
>tm_year, >tm_mon, >tm_mday,
>tm_hour, >tm_min, >tm_sec);
- pr_debug("%s: read %04d-%02d-%-2d %02d:%02d:%02d\n",
-__func__, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday,
-t->tm_hour, t->tm_min, t->tm_sec);
+ pr_debug("%s: read %ptR\n", __func__, t);
} else { /* write */
- pr_debug("%s: tried to write %04d-%02d-%-2d %02d:%02d:%02d\n",
-__func__, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday,
-t->tm_hour, t->tm_min, t->tm_sec);
+ pr_debug("%s: tried to write %ptR\n", __func__, t);
switch (macintosh_config->adb_type) {
case MAC_ADB_IOP:
--
2.19.2
[PATCH v5 20/21] m68k/mac: Switch to use %ptR
Use %ptR instead of open coded variant to print content of
struct rtc_time in human readable format.
Cc: Geert Uytterhoeven
Cc: linux-m68k
Signed-off-by: Andy Shevchenko
---
arch/m68k/mac/misc.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/m68k/mac/misc.c b/arch/m68k/mac/misc.c
index ebb3b6d169ea..71c4735a31ee 100644
--- a/arch/m68k/mac/misc.c
+++ b/arch/m68k/mac/misc.c
@@ -605,13 +605,9 @@ int mac_hwclk(int op, struct rtc_time *t)
unmktime(now, 0,
>tm_year, >tm_mon, >tm_mday,
>tm_hour, >tm_min, >tm_sec);
- pr_debug("%s: read %04d-%02d-%-2d %02d:%02d:%02d\n",
-__func__, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday,
-t->tm_hour, t->tm_min, t->tm_sec);
+ pr_debug("%s: read %ptR\n", __func__, t);
} else { /* write */
- pr_debug("%s: tried to write %04d-%02d-%-2d %02d:%02d:%02d\n",
-__func__, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday,
-t->tm_hour, t->tm_min, t->tm_sec);
+ pr_debug("%s: tried to write %ptR\n", __func__, t);
switch (macintosh_config->adb_type) {
case MAC_ADB_IOP:
--
2.19.2
Re: [PATCH] clk: Add (devm_)clk_get_optional() functions
On Tue, Nov 20, 2018 at 01:56:52PM +0100, Uwe Kleine-König wrote: > On Tue, Nov 20, 2018 at 12:38:33PM +0200, Andy Shevchenko wrote: > > On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote: > > > + if (clk == ERR_PTR(-ENOENT)) > > > + return NULL; > > > + else > > > + return clk; > > > > return clk == ERR_PTR(-ENOENT) ? NULL : clk; > > > > ? > > Not sure this adds to the readability of the expression. Personally I > prefer the explicit if. Maybe even: > > clk = clk_get(...); > > if (clk == ERR_PTR(-ENOENT)) > clk = NULL; > > return clk; So, it almost repeats the initial variant. I'm fine with no 'else' in initial code, like if (...) return NULL; return clk; -- With Best Regards, Andy Shevchenko
Re: [PATCH] clk: Add (devm_)clk_get_optional() functions
On Tue, Nov 20, 2018 at 10:53:33AM +, Phil Edworthy wrote: > On 20 November 2018 10:39 Andy Shevchenko wrote: > > On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote: > > > This adds clk_get_optional() and devm_clk_get_optional() functions to > > > get optional clocks. > > > They behave the same as (devm_)clk_get except where there is no clock > > > producer. In this case, instead of returning -ENOENT, the function > > > returns NULL. This makes error checking simpler and allows > > > clk_prepare_enable, etc to be called on the returned reference without > > > additional checks. > > > - Instead of messing with the core functions, simply wrap them for the > > >_optional() versions. By putting clk_get_optional() inline in the > > > header > > >file, we can get rid of the arch specific patches as well. > > Fine if it would have no surprises with error handling. > There shouldn't be any surprises. My earlier attempts at implementing this > were hampered by the fact that of_clk_get_by_name() can return -EINVAL > in some circumstances. By directly wrapping the (devm_)clk_get() functions > that problem goes away. Good! After my comments being addressed, Reviewed-by: Andy Shevchenko > > > + if (ERR_PTR(-ENOENT)) > Huh? That wasn't the code I sent... Yup, it's my wrong editing flow. Anyway, you got the idea. > > > + return NULL; > > > + else > > > + return clk; > > return clk == ERR_PTR(-ENOENT) ? NULL : clk; -- With Best Regards, Andy Shevchenko
Re: [PATCH] clk: Add (devm_)clk_get_optional() functions
On Mon, Nov 19, 2018 at 02:12:59PM +, Phil Edworthy wrote: > This adds clk_get_optional() and devm_clk_get_optional() functions to get > optional clocks. > They behave the same as (devm_)clk_get except where there is no clock > producer. In this case, instead of returning -ENOENT, the function > returns NULL. This makes error checking simpler and allows > clk_prepare_enable, etc to be called on the returned reference > without additional checks. > - Instead of messing with the core functions, simply wrap them for the >_optional() versions. By putting clk_get_optional() inline in the header >file, we can get rid of the arch specific patches as well. Fine if it would have no surprises with error handling. > + if (ERR_PTR(-ENOENT)) > + return NULL; > + else > + return clk; return clk == ERR_PTR(-ENOENT) ? NULL : clk; ? > + if (clk == ERR_PTR(-ENOENT)) > + return NULL; > + else > + return clk; Ditto. -- With Best Regards, Andy Shevchenko
Re: [PATCH] m68k: Implement ndelay() as an inline function to force type checking/casting
On Sun, May 13, 2018 at 5:02 PM, Boris Brezillon
<boris.brezil...@bootlin.com> wrote:
> ndelay() is supposed to take an unsigned long, but if you define
> ndelay() as a macro and the caller pass an unsigned long long instead
> of an unsigned long, the unsigned long long to unsigned long cast is
> not done and we end up with an "undefined reference to `__udivdi3'"
> error at link time.
> -#define ndelay(n) __delay(DIV_ROUND_UP((n) * HZSCALE) >> 11) *
> (loops_per_jiffy >> 11)) >> 6), 1000))
> +static inline void ndelay(unsigned long nsec)
> +{
> + __delay(DIV_ROUND_UP(nsec *
> +HZSCALE) >> 11) *
One pair of parens is redundant.
> + (loops_per_jiffy >> 11)) >> 6),
> +1000));
Can't you keep as one line as in original?
> +}
> +#define ndelay(n) ndelay(n)
>
> #endif /* defined(_M68K_DELAY_H) */
> --
> 2.14.1
>
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ata: add Amiga Gayle PATA controller driver
On Thu, Mar 1, 2018 at 1:02 PM, Bartlomiej Zolnierkiewicz
<b.zolnier...@samsung.com> wrote:
> Add Amiga Gayle PATA controller driver. It enables libata support
> for the on-board IDE interfaces on some Amiga models (A600, A1200,
> A4000 and A4000T) and also for IDE interfaces on the Zorro expansion
> bus (M-Tech E-Matrix 530 expansion card).
>
> Thanks to John Paul Adrian Glaubitz and Michael Schmitz for help
> with testing the driver.
> +#include
> +#include
> +#include
Choose one of those two.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Better to keep in order.
> +#include
> +#include
> +#include
> +#include
> +#include
Ditto.
> +static bool pata_gayle_irq_check(struct ata_port *ap)
> +{
> + u8 ch;
> +
> + ch = z_readb((unsigned long)ap->private_data);
> + if (!(ch & GAYLE_IRQ_IDE))
> + return false;
> + return true;
return !!(ch & GAYLE_IRQ_IDE);
?
> +}
> +static int __init pata_gayle_init_one(struct platform_device *pdev)
> +{
> + pr_info(DRV_NAME ": Amiga Gayle IDE controller (A%u style)\n",
> + pdata->explicit_ack ? 1200 : 4000);
What's wrong with dev_info() ?
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + if (!devm_request_mem_region(>dev, res->start,
> +resource_size(res), DRV_NAME)) {
> + pr_err(DRV_NAME ": resources busy\n");
> + return -EBUSY;
> + }
> +
> + base = ZTWO_VADDR(pdata->base);
Hmm... Can't you use devm_ioremap_resources() to get the virtual
address for I/O ?
> + ap->private_data = (void *)ZTWO_VADDR(pdata->irqport);
> +
> + ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
> + (unsigned long)base + GAYLE_CONTROL);
When you use explicit casting in printf() you are doing in 99.9% cases
something wrong.
> + /* activate */
Noise.
> + ret = ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
> + IRQF_SHARED, _gayle_sht);
> + if (ret)
> + return ret;
> +}
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/7] x86: put msr-index.h in uapi
On Fri, Jan 6, 2017 at 11:43 AM, Nicolas Dichtel <nicolas.dich...@6wind.com> wrote: > This header file is exported, thus move it to uapi. Just hint for the future: -M (move) -C (copy) -D (delete) [though this is NOT for applying] -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
