Re: [PATCH] scsi: a100u2w: remove useless variable

2021-04-17 Thread Julian Calaby
Hi Jiapeng,

On Mon, Apr 12, 2021 at 1:23 PM Jiapeng Chong
 wrote:
>
> Fix the following gcc warning:
>
> drivers/scsi/a100u2w.c:1092:8: warning: variable ‘bios_phys’ set but not
> used [-Wunused-but-set-variable].
>
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 
> ---
>  drivers/scsi/a100u2w.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/a100u2w.c b/drivers/scsi/a100u2w.c
> index 66c5143..855a3fe 100644
> --- a/drivers/scsi/a100u2w.c
> +++ b/drivers/scsi/a100u2w.c
> @@ -1089,7 +1089,6 @@ static int inia100_probe_one(struct pci_dev *pdev,
> int error = -ENODEV;
> u32 sz;
> unsigned long biosaddr;
> -   char *bios_phys;
>
> if (pci_enable_device(pdev))
> goto out;
> @@ -1141,7 +1140,7 @@ static int inia100_probe_one(struct pci_dev *pdev,
>
> biosaddr = host->BIOScfg;
> biosaddr = (biosaddr << 4);
> -   bios_phys = phys_to_virt(biosaddr);
> +   phys_to_virt(biosaddr);

Does phys_to_virt() have side effects? If it doesn't, there's a lot
more stuff that can get deleted here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: scsi: Describe and replace a word with better one in the file qlogicpti.h

2021-02-05 Thread Julian Calaby
Hi Bhaskar,

On Sat, Feb 6, 2021 at 9:55 AM Bhaskar Chowdhury  wrote:
>
>
>
> s/fucking/awful/
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/scsi/qlogicpti.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qlogicpti.h b/drivers/scsi/qlogicpti.h
> index 2b6374e08a7d..4a47631b22ea 100644
> --- a/drivers/scsi/qlogicpti.h
> +++ b/drivers/scsi/qlogicpti.h
> @@ -62,7 +62,7 @@
>  #define REQUEST_QUEUE_WAKEUP   0x8005
>  #define EXECUTION_TIMEOUT_RESET0x8006
>
> -/* Am I fucking pedantic or what? */
> +/* Am I awful pedantic or what? */

You're right that this needs to go, but "awfully" is a better
replacement than "awful".

However it's likely that the entire comment can either be removed or
replaced with something more descriptive.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] b43: Remove redundant code

2021-01-28 Thread Julian Calaby
Hi . ,

(No proper name in the from field or signed-off-by, as you're already aware)

On Thu, Jan 28, 2021 at 7:53 PM Abaci Team
 wrote:
>
> Fix the following coccicheck warnings:
>
> ./drivers/net/wireless/broadcom/b43/phy_n.c:4640:2-4: WARNING: possible
> condition with no effect (if == else).
>
> ./drivers/net/wireless/broadcom/b43/phy_n.c:4606:2-4: WARNING: possible
> condition with no effect (if == else).
>
> Reported-by: Abaci Robot 
> Suggested-by: Jiapeng Zhong 
> Signed-off-by: Abaci Team 
> ---
>  drivers/net/wireless/broadcom/b43/phy_n.c | 16 
>  1 file changed, 16 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
> b/drivers/net/wireless/broadcom/b43/phy_n.c
> index b669dff..39a335f 100644
> --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> @@ -4601,16 +4601,6 @@ static void b43_nphy_spur_workaround(struct b43_wldev 
> *dev)
> if (nphy->hang_avoid)
> b43_nphy_stay_in_carrier_search(dev, 1);
>
> -   if (nphy->gband_spurwar_en) {
> -   /* TODO: N PHY Adjust Analog Pfbw (7) */
> -   if (channel == 11 && b43_is_40mhz(dev)) {
> -   ; /* TODO: N PHY Adjust Min Noise Var(2, tone, 
> noise)*/
> -   } else {
> -   ; /* TODO: N PHY Adjust Min Noise Var(0, NULL, NULL)*/
> -   }
> -   /* TODO: N PHY Adjust CRS Min Power (0x1E) */
> -   }

I'm not sure how useful this patch is, even though it is technically correct.

The b43 driver was almost entirely reverse engineered from various
sources so there's still a lot of places, like this, where placeholder
comments were written until the actual code that would have been here
was ready / reverse engineered.

That said, I believe the driver works well enough for all it's users
and has not seen any significant changes in a long time.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

On Thu, Jan 28, 2021 at 7:53 PM Abaci Team
 wrote:
>
> Fix the following coccicheck warnings:
>
> ./drivers/net/wireless/broadcom/b43/phy_n.c:4640:2-4: WARNING: possible
> condition with no effect (if == else).
>
> ./drivers/net/wireless/broadcom/b43/phy_n.c:4606:2-4: WARNING: possible
> condition with no effect (if == else).
>
> Reported-by: Abaci Robot 
> Suggested-by: Jiapeng Zhong 
> Signed-off-by: Abaci Team 
> ---
>  drivers/net/wireless/broadcom/b43/phy_n.c | 16 
>  1 file changed, 16 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
> b/drivers/net/wireless/broadcom/b43/phy_n.c
> index b669dff..39a335f 100644
> --- a/drivers/net/wireless/broadcom/b43/phy_n.c
> +++ b/drivers/net/wireless/broadcom/b43/phy_n.c
> @@ -4601,16 +4601,6 @@ static void b43_nphy_spur_workaround(struct b43_wldev 
> *dev)
> if (nphy->hang_avoid)
> b43_nphy_stay_in_carrier_search(dev, 1);
>
> -   if (nphy->gband_spurwar_en) {
> -   /* TODO: N PHY Adjust Analog Pfbw (7) */
> -   if (channel == 11 && b43_is_40mhz(dev)) {
> -   ; /* TODO: N PHY Adjust Min Noise Var(2, tone, 
> noise)*/
> -   } else {
> -   ; /* TODO: N PHY Adjust Min Noise Var(0, NULL, NULL)*/
> -   }
> -   /* TODO: N PHY Adjust CRS Min Power (0x1E) */
> -   }
> -
> if (nphy->aband_spurwar_en) {
> if (channel == 54) {
> tone[0] = 0x20;
> @@ -4636,12 +4626,6 @@ static void b43_nphy_spur_workaround(struct b43_wldev 
> *dev)
> tone[0] = 0;
> noise[0] = 0;
> }
> -
> -   if (!tone[0] && !noise[0]) {
> -   ; /* TODO: N PHY Adjust Min Noise Var(1, tone, 
> noise)*/
> -   } else {
> -   ; /* TODO: N PHY Adjust Min Noise Var(0, NULL, NULL)*/
> -   }
> }
>
> if (nphy->hang_avoid)
> --
> 1.8.3.1
>


-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [RFC PATCH 00/37] block: introduce bio_init_fields()

2021-01-20 Thread Julian Calaby
Hi Chaitanya,

On Tue, Jan 19, 2021 at 5:01 PM Chaitanya Kulkarni
 wrote:
>
> Hi,
>
> This is a *compile only RFC* which adds a generic helper to initialize
> the various fields of the bio that is repeated all the places in
> file-systems, block layer, and drivers.
>
> The new helper allows callers to initialize various members such as
> bdev, sector, private, end io callback, io priority, and write hints.
>
> The objective of this RFC is to only start a discussion, this it not
> completely tested at all.
> Following diff shows code level benefits of this helper :-
>  38 files changed, 124 insertions(+), 236 deletions(-)

On a more abstract note, I don't think this diffstat is actually
illustrating the benefits of this as much as you think it is.

Yeah, we've reduced the code by 112 lines, but that's barely half the
curn here. It looks, from the diffstat, that you've effectively
reduced 2 lines into 1. That isn't much of a saving.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: rtlwifi: rtl8723ae: Fix word association in trx.c

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:39 PM Bhaskar Chowdhury  wrote:
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c 
> b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> index 340b3d68a54e..59e0a04b167d 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> @@ -673,7 +673,7 @@ bool rtl8723e_is_tx_desc_closed(struct ieee80211_hw *hw,
>
> /**
>  *beacon packet will only use the first
> -*descriptor defautly,and the own may not
> +*descriptor de-faulty,and the own may not

Same comments here as the previous patches:

"de-faultly" makes less sense than "defaultly". This comment needs to
be re-written by someone who knows what's going on here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: realtek: rtl8723be: Correct word presentation as defautly to de-faulty

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:44 PM Bhaskar Chowdhury  wrote:
>
> s/defautly/de-faulty/p
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.c 
> b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.c
> index 5a7cd270575a..47886a19ed8c 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.c
> @@ -724,7 +724,7 @@ bool rtl8723be_is_tx_desc_closed(struct ieee80211_hw *hw,
> u8 own = (u8)rtl8723be_get_desc(hw, entry, true, HW_DESC_OWN);
>
> /*beacon packet will only use the first
> -*descriptor defautly,and the own may not
> +*descriptor de-faulty,and the own may not

Same comments here as the previous patches:

"de-faultly" makes less sense than "defaultly". This comment needs to
be re-written by someone who knows what's going on here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: rtlwifi: rtl8821ae: defautly to de-faulty ,last in the series

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:48 PM Bhaskar Chowdhury  wrote:
>
> s/defautly/de-faulty/p
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.c 
> b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.c
> index 9d6f8dcbf2d6..0e8f7c5fd028 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.c
> @@ -970,7 +970,7 @@ bool rtl8821ae_is_tx_desc_closed(struct ieee80211_hw *hw,
>
> /**
>  *beacon packet will only use the first
> -*descriptor defautly,and the own may not
> +*descriptor de-faulty,and the own may not

Same comments here as the previous patches:

"de-faultly" makes less sense than "defaultly". This comment needs to
be re-written by someone who knows what's going on here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: net: wireless: realtek: Fix the word association defautly de-faulty

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:48 PM Bhaskar Chowdhury  wrote:
>
> On 21:33 Tue 05 Jan 2021, Julian Calaby wrote:
> >Hi Bhaskar,
> >
> >On Tue, Jan 5, 2021 at 9:19 PM Bhaskar Chowdhury  
> >wrote:
> >>
> >> s/defautly/de-faulty/p
> >>
> >>
> >> Signed-off-by: Bhaskar Chowdhury 
> >> ---
> >>  drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c 
> >> b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> >> index c948dafa0c80..7d02d8abb4eb 100644
> >> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> >> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> >> @@ -814,7 +814,7 @@ bool rtl88ee_is_tx_desc_closed(struct ieee80211_hw 
> >> *hw, u8 hw_queue, u16 index)
> >> u8 own = (u8)rtl88ee_get_desc(hw, entry, true, HW_DESC_OWN);
> >>
> >> /*beacon packet will only use the first
> >> -*descriptor defautly,and the own may not
> >> +*descriptor de-faulty,and the own may not
> >
> >Really? "de-faultly" isn't any better than "defaultly" and in fact
> >it's even worse as it breaks up the word "default".
> >
> hey, it was written as "defautly" ...and that was simple spelling mistake ..
> so,corrected it.

Er, no, that isn't the correct replacement. They're using "default" as
a verb and mean "by default".

The sentence makes no sense with "de-faulty" there instead.

Ultimately though the entire comment barely makes sense, so the best
way to fix this spelling mistake is to re-write the entire comment so
it does. I would have suggested a new wording for it, but I don't know
enough about what's going on here to parse the rest of it.

So therefore someone who knows what's going on here needs to fix this
and your change is just making this comment worse.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: wireless: rtlwifi: rtl8192ce: Fix construction of word rtl8192ce/trx.c

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:32 PM Bhaskar Chowdhury  wrote:
>
> s/defautly/de-faulty/p
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8192ce/trx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/trx.c 
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/trx.c
> index 4165175cf5c0..d53397e7eb2e 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/trx.c
> @@ -671,7 +671,7 @@ bool rtl92ce_is_tx_desc_closed(struct ieee80211_hw *hw,
> u8 own = (u8)rtl92ce_get_desc(hw, entry, true, HW_DESC_OWN);
>
> /*beacon packet will only use the first
> -*descriptor defautly,and the own may not
> +*descriptor de-faulty,and the own may not

Same comments here as the previous patch:

"de-faultly" makes less sense than "defaultly". This comment needs to
be re-written by someone who knows what's going on here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] drivers: net: wireless: realtek: Fix the word association defautly de-faulty

2021-01-05 Thread Julian Calaby
Hi Bhaskar,

On Tue, Jan 5, 2021 at 9:19 PM Bhaskar Chowdhury  wrote:
>
> s/defautly/de-faulty/p
>
>
> Signed-off-by: Bhaskar Chowdhury 
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c 
> b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> index c948dafa0c80..7d02d8abb4eb 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/trx.c
> @@ -814,7 +814,7 @@ bool rtl88ee_is_tx_desc_closed(struct ieee80211_hw *hw, 
> u8 hw_queue, u16 index)
> u8 own = (u8)rtl88ee_get_desc(hw, entry, true, HW_DESC_OWN);
>
> /*beacon packet will only use the first
> -*descriptor defautly,and the own may not
> +*descriptor de-faulty,and the own may not

Really? "de-faultly" isn't any better than "defaultly" and in fact
it's even worse as it breaks up the word "default".

This change doesn't make sense and the comment really needs to be
completely re-written by someone who understands what's going on here
as it barely makes sense.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [RFC PATCH 0/13] sparc32: sunset sun4m and sun4d

2020-12-20 Thread Julian Calaby
Hi Romain,

On Sun, Dec 20, 2020 at 8:26 PM Romain Dolbeau  wrote:
>
> Le dim. 20 déc. 2020 à 09:54, Julian Calaby  a écrit 
> :
> > If I want to run them, assuming the hardware still works, I need to
> > netboot them as I cannot find working, compatible HDDs for them as
> > everything has switched to SATA or SAS.
>
> SCSI2SD (<http://www.codesrc.com/mediawiki/index.php/SCSI2SD>)
> are a bit expensive, but solve that problem (I own both a V5 and a V6,
> both work well in my SPARCstations, tried sun4c and sun4m).
> As it takes micro-sd cards, it's quite easy to keep multiple OSes
> on hand.

I'd forgotten about that. Fair point =)

> > Then there's the issue of finding a monitor as they're not
> > electrically compatible with VGA
>
> Huh? There is Sun's 13W3-to-vga adapters and cables, and many
> monitors will sync to Sun's frequency (though not the most recent
> LCDs whose analog circuitry is pathetic compared to old-school
> CRTs). Some framebuffers will output 1280x1024 (rarer than for
> 1152x900), and some can be coerced to do almost anything with
> some Forth knowledge (see e.g.
> <https://github.com/rdolbeau/SunTurboGX>, again blowing my
> own horn here sorry...).

Yeah, my issue is that I have no CRTs anymore - all my monitors are
LCDs and none of them sync to the frequencies SUNs use.

So yeah, you can make adapters (i have home-made adapters to convert
both ways) however out of the 4 monitors I own with VGA ports, none of
them sync to Sun frequencies.

> > (...) booting one up for fun is simply impractical
>
> An SCSI2SD and either a null-modem serial cable or a
> Sun keyboard/13w3 cable/17"LCD combo and you're good to
> go. You might need another unix-like box to netboot the system.

That's almost exactly what I was planning to do, but I'd still be
lacking a Linux distro to run.

> > I believe that Gentoo is architecture-neutral enough that it'd work,
> > but I believe that you'll have to compile everything - there'll be no
> > pre-built anything for sparc32
>
> Trying gentoo is on my todo list... has been for a long time :-(

Same.

IIRC there's some ancient versions that have the bits to netboot a
SparcStation so you can then do the necessary stuff to install the
minimal binaries and stuff, at which point you can do the various
configurations, pull in the latest portage tree and emerge world,
however the last time I tried this, the disk I was using - my last one
- failed somewhere in the middle of that process.

> > and as it's fairly slow hardware by
> > today's standards, that's going to take a long time, however you could
> > probably use distcc and cross-compilers to speed it up.
>
> Isn't that what Qemu is for ? :-) I've managed to recompile LLVM
> and clang in NetBSD 9 for my SS20, one by cross-compiling
> (LLVM requires too much memory), the other in QEmu.
> Unfortunately, Qemu doesn't yet support mt-tcg (multithreaded
> emulation) for sparc so single-core only - still faster than the HW,
> mostly because of incomparably faster I/O.

My distcc plan was to have it talk to a cross compiler on my x86
desktop. I never got to the point where it would have actually used
it.

> > If there were more people using it or more testing, or more distros
> > supporting it - not just (theoretically?) working on it - then I'd be
> > fighting to keep it.
>
> I wish I had some arguments for that point... I will just re-mention Qemu,
> as it makes testing quite easy and reasonably not-too-slow.

QEMU is somewhat slow and never _exactly_ the same as real hardware,
so I can see why distros might not use it as a build machine or
whatever. And if they do build for QEMU and it doesn't work on real
hardware, then we have a distro that's only for virtual hardware and
that seems pointless.

You're right that with the right bits and pieces resurrecting a
Sparc32 machine is relatively "easy", however there's still no modern
distros supporting this ancient hardware so the upstream kernel has
most likely bitrotted.

I still don't think it's worth saving.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [RFC PATCH 0/13] sparc32: sunset sun4m and sun4d

2020-12-20 Thread Julian Calaby
Hi Romain, Sam,

On Sun, Dec 20, 2020 at 6:46 PM Romain Dolbeau  wrote:
>
> Le sam. 19 déc. 2020 à 22:41, Sam Ravnborg  a écrit :
> > Another said that it would be a shame to sunset sun4m and sun4d because
> > there are so many machines around, and netbsd is also active on the
> > sparc32 area.
>
> Yes, those were plentiful back in the day and there's still quite a few 
> around.

I have three: two SparcStation 10s and a SparcStation LX.

If I want to run them, assuming the hardware still works, I need to
netboot them as I cannot find working, compatible HDDs for them as
everything has switched to SATA or SAS.

Then there's the issue of finding a monitor as they're not
electrically compatible with VGA and I'm pretty sure none of the VGA
compatible monitors I have or can lay hands on works with their
specific sync frequencies.

Ultimately it's one of those things where there's enough "stuff" in
the way that booting one up for fun is simply impractical and they're
old and slow enough that they're not useful for anything else.

Then we get to the not-so-significant issue of software...

> > The second mail also re-reminded me of an interesting project
> > implementing SPARC V8 and the sun4m platform in VHDL.
>
> There's also new hardware being developed for SBus systems :-)
> <https://github.com/rdolbeau/SBusFPGA>
> (disclaimer: work-in-progress and shameless self-promotion here!).

Interesting project!

Amusingly enough you're not the first to hook an FPGA up to sBus. I
had a card that was some form of high-speed sampling thing which was
effectively some electrically isolated front-end hooked to a Xylinx
FPGA. I ended up trashing it as it had no markings and I couldn't find
out anything about it.

> If there's still a distribution willing to build for Sparc v8, then I
> believe the kernel
> should try to keep support of the relevant machine architectures if at all
> possible...

And here's where the problem lies. The last (official) version of
Debian to support Sparc32 was Etch and I believe it was one of the
last ones to drop support.

I believe that Gentoo is architecture-neutral enough that it'd work,
but I believe that you'll have to compile everything - there'll be no
pre-built anything for sparc32 - and as it's fairly slow hardware by
today's standards, that's going to take a long time, however you could
probably use distcc and cross-compilers to speed it up.

Long painful story short, it's difficult to get the hardware running,
there's practically no Linux distros that support it, and the kernel
code has probably bitrotted due to lack of testing.

As much as it pains me to say this, I think this code's time has come
and it's time to get rid of it.

If there were more people using it or more testing, or more distros
supporting it - not just (theoretically?) working on it - then I'd be
fighting to keep it.

But there isn't.

I think it's time for it to go.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

On Sun, Dec 20, 2020 at 6:46 PM Romain Dolbeau  wrote:
>
> Le sam. 19 déc. 2020 à 22:41, Sam Ravnborg  a écrit :
> > Another said that it would be a shame to sunset sun4m and sun4d because
> > there are so many machines around, and netbsd is also active on the
> > sparc32 area.
>
> Yes, those were plentiful back in the day and there's still quite a few 
> around.
>
> > The second mail also re-reminded me of an interesting project
> > implementing SPARC V8 and the sun4m platform in VHDL.
>
> There's also new hardware being developed for SBus systems :-)
> <https://github.com/rdolbeau/SBusFPGA>
> (disclaimer: work-in-progress and shameless self-promotion here!).
>
> If there's still a distribution willing to build for Sparc v8, then I
> believe the kernel
> should try to keep support of the relevant machine architectures if at all
> possible...
>
> Cordially,
>
> --
> Romain Dolbeau



-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 1/1] mwifiex: Fix possible buffer overflows in mwifiex_config_scan

2020-12-08 Thread Julian Calaby
Hi Xiaohui,

On Wed, Dec 9, 2020 at 12:07 AM Xiaohui Zhang  wrote:
>
> From: Zhang Xiaohui 
>
> mwifiex_config_scan() calls memcpy() without checking
> the destination size may trigger a buffer overflower,
> which a local user could use to cause denial of service
> or the execution of arbitrary code.
> Fix it by putting the length check before calling memcpy().
>
> Signed-off-by: Zhang Xiaohui 
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
> b/drivers/net/wireless/marvell/mwifiex/scan.c
> index c2a685f63..b1d90678f 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -930,6 +930,8 @@ mwifiex_config_scan(struct mwifiex_private *priv,
> "DIRECT-", 7))
> wildcard_ssid_tlv->max_ssid_length = 0xfe;
>
> +   if (ssid_len > 1)
> +   ssid_len = 1;
> memcpy(wildcard_ssid_tlv->ssid,
>user_scan_in->ssid_list[i].ssid, ssid_len);

Can ssid_len ever be 0 here?

If it can't, should we just set ssid_len to 1 unconditionally?

If it can, should we just skip the memcpy as it won't do anything?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH v2] ath10k: sdio: remove redundant check in for loop

2020-09-16 Thread Julian Calaby
Hi Alex,

On Thu, Sep 17, 2020 at 3:09 AM Alex Dewar  wrote:
>
> The for loop checks whether cur_section is NULL on every iteration, but
> we know it can never be NULL as there is another check towards the
> bottom of the loop body. Refactor to avoid this unnecessary check.
>
> Also, increment the variable i inline for clarity

Comments below.

> Addresses-Coverity: 1496984 ("Null pointer dereferences)
> Suggested-by: Saeed Mahameed 
> Signed-off-by: Alex Dewar 
> ---
> v2: refactor in the manner suggested by Saeed
>
>  drivers/net/wireless/ath/ath10k/sdio.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c 
> b/drivers/net/wireless/ath/ath10k/sdio.c
> index 81ddaafb6721..486886c74e6a 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -2307,8 +2307,8 @@ static int ath10k_sdio_dump_memory_section(struct 
> ath10k *ar,
> }
>
> count = 0;
> -
> -   for (i = 0; cur_section; i++) {
> +   i = 0;
> +   for (; cur_section; cur_section = next_section) {

You can have multiple statements in each section of a for() if you need to, e.g.

for (i = 1; cur_section; cur_section = next_section, i++) {

which means that the increment of i isn't hidden deep in the function body.


That said, this function is a mess. Something (approximately) like
this might be more readable:

prev_end = memregion->start;
for (i = 0; i < mem_region->section_table.size; i++) {
cur_section = _region->section_table.sections[i];

// fail if prev_end is greater than cur_section->start - message
from line 2329 and 2294
// check section size - from line 2315

skip_size = cur_section->start - prev_end;

// check buffer size - from line 2339 - needs to account for the
skip size too.
// fill in the skip size amount - from line 2358 and 2304
// ath10k_sdio_read_mem - from line 2346

prev_end = cur_section->end;
}

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH][next] ath11k: Use fallthrough pseudo-keyword

2020-07-27 Thread Julian Calaby
Hi Joe,

On Tue, Jul 28, 2020 at 5:48 AM Joe Perches  wrote:
>
> On Mon, 2020-07-27 at 14:44 -0500, Gustavo A. R. Silva wrote:
> > Replace the existing /* fall through */ comments and its variants with
> > the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
> > fall-through markings when it is the case.
> []
> > diff --git a/drivers/net/wireless/ath/ath11k/dp.c 
> > b/drivers/net/wireless/ath/ath11k/dp.c
> []
> > @@ -159,7 +159,7 @@ int ath11k_dp_srng_setup(struct ath11k_base *ab, struct 
> > dp_srng *ring,
> >   break;
> >   }
> >   /* follow through when ring_num >= 3 */
> > - /* fall through */
> > + fallthrough;
>
> Likely the /* follow through ... */ comment can be deleted too

If the "when ring_num >= 3" comment is needed, how should this get
formatted? Maybe something like:

fallthrough; /* when ring_num >= 3 */

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] sparc: sparc64_defconfig: add necessary configs for qemu

2020-07-02 Thread Julian Calaby
Hi Corentin,

On Thu, Jul 2, 2020 at 11:03 PM Corentin Labbe  wrote:
>
> The sparc64 qemu machines uses pcnet32 network hardware by default, so for
> simple boot testing using qemu, having PCNET32 is useful.
> Same for its storage which is a PATA_CMD64.
>
> Signed-off-by: Corentin Labbe 
> ---
>  arch/sparc/configs/sparc64_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/sparc/configs/sparc64_defconfig 
> b/arch/sparc/configs/sparc64_defconfig
> index bde4d21a8ac8..61073f48a7a1 100644
> --- a/arch/sparc/configs/sparc64_defconfig
> +++ b/arch/sparc/configs/sparc64_defconfig
> @@ -236,3 +236,6 @@ CONFIG_CRYPTO_TWOFISH=m
>  CONFIG_CRC16=m
>  CONFIG_LIBCRC32C=m
>  CONFIG_VCC=m
> +CONFIG_ATA=y
> +CONFIG_PATA_CMD64X=y
> +CONFIG_PCNET32=y

FWIW the CMD640 is the IDE controller used on the Ultra 5/10 machines.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] Revert "ath: add support for special 0x0 regulatory domain"

2020-05-28 Thread Julian Calaby
Hi Brian,

On Thu, May 28, 2020 at 5:18 AM Brian Norris  wrote:
>
> This reverts commit 2dc016599cfa9672a147528ca26d70c3654a5423.
>
> Users are reporting regressions in regulatory domain detection and
> channel availability.
>
> The problem this was trying to resolve was fixed in firmware anyway:

Should we tell the user their firmware needs to be upgraded if it
reports this regulatory domain instead of completely dropping support
for it?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH v3] lsilogic mpt fusion: mptctl: Fixed race condition around mptctl_id variable using mutexes

2019-08-18 Thread Julian Calaby
Hi Mark,

On Thu, Aug 15, 2019 at 8:02 PM Mark Balantzyan  wrote:
>
> Certain functions in the driver, such as mptctl_do_fw_download() and
> mptctl_do_mpt_command(), rely on the instance of mptctl_id, which does the
> id-ing. There is race condition possible when these functions operate in
> concurrency. Via, mutexes, the functions are mutually signalled to cooperate.
>
> Changelog v2
>
> Lacked a version number but added properly terminated else condition at
> (former) line 2300.
>
> Changelog v3
>
> Fixes "return -EAGAIN" lines which were erroneously tabbed as if to be guarded
> by "if" conditions lying above them.
>
> Signed-off-by: Mark Balantzyan 
>
> ---

Changelog should be down here after the "---"

>  drivers/message/fusion/mptctl.c | 43 +
>  1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
> index 4470630d..3270843c 100644
> --- a/drivers/message/fusion/mptctl.c
> +++ b/drivers/message/fusion/mptctl.c
> @@ -816,12 +816,15 @@ mptctl_do_fw_download(int ioc, char __user *ufwbuf, 
> size_t fwlen)
>
> /*  Valid device. Get a message frame and construct the FW 
> download message.
> */
> +   mutex_lock(_mutex);
> if ((mf = mpt_get_msg_frame(mptctl_id, iocp)) == NULL)
> -   return -EAGAIN;
> +   mutex_unlock(_mutex);
> +   return -EAGAIN;

Are you sure this is right?

1. We're now exiting early with -EAGAIN regardless of the result of
mpt_get_msg_frame()
2. If the result of mpt_get_msg_frame() is not NULL, we don't unlock the mutex

Do you mean something like:

- - - - - -

mutex_lock(_mutex);
mf = mpt_get_msg_frame(mptctl_id, iocp);
mutex_unlock(_mutex);

if (mf == NULL) {

- - - - - -

> @@ -1889,8 +1894,10 @@ mptctl_do_mpt_command (struct mpt_ioctl_command karg, 
> void __user *mfPtr)
>
> /* Get a free request frame and save the message context.
>  */
> +   mutex_lock(_mutex);
>  if ((mf = mpt_get_msg_frame(mptctl_id, ioc)) == NULL)
> -return -EAGAIN;
> +   mutex_unlock(_mutex);
> +return -EAGAIN;

Same comments here.

> @@ -2563,7 +2576,9 @@ mptctl_hp_hostinfo(unsigned long arg, unsigned int 
> data_size)
> /*
>  * Gather ISTWI(Industry Standard Two Wire Interface) Data
>  */
> +   mutex_lock(_mutex);
> if ((mf = mpt_get_msg_frame(mptctl_id, ioc)) == NULL) {
> +   mutex_unlock(_mutex);

This line needs to be indented to match the line below, also we don't
unlock the mutex if mpt_get_msg_frame() is not NULL.

> @@ -3010,9 +3027,11 @@ static int __init mptctl_init(void)
>  *  Install our handler
>  */
> ++where;
> +   mutex_lock(_mutex);
> mptctl_id = mpt_register(mptctl_reply, MPTCTL_DRIVER,
> "mptctl_reply");
> if (!mptctl_id || mptctl_id >= MPT_MAX_PROTOCOL_DRIVERS) {
> +   mutex_unlock(_mutex);

Why not use a local variable and only update the global variable if it's valid?

> printk(KERN_ERR MYNAM ": ERROR: Failed to register with 
> Fusion MPT base driver\n");
> misc_deregister(_miscdev);
> err = -EBUSY;
> @@ -3022,13 +3041,14 @@ static int __init mptctl_init(void)
> mptctl_taskmgmt_id = mpt_register(mptctl_taskmgmt_reply, 
> MPTCTL_DRIVER,
> "mptctl_taskmgmt_reply");
> if (!mptctl_taskmgmt_id || mptctl_taskmgmt_id >= 
> MPT_MAX_PROTOCOL_DRIVERS) {
> +   mutex_unlock(_mutex);

Same comment here.

> @@ -3044,13 +3064,14 @@ out_fail:
>  
> /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
>  static void mptctl_exit(void)
>  {
> +   mutex_lock(_mutex);
> misc_deregister(_miscdev);
> printk(KERN_INFO MYNAM ": Deregistered /dev/%s @ 
> (major,minor=%d,%d)\n",
>  mptctl_miscdev.name, MISC_MAJOR, 
> mptctl_miscdev.minor);
>
> /* De-register event handler from base module */
> mpt_event_deregister(mptctl_id);
> -
> +

Please don't add trailing whitespace.

Did you test this on real hardware? I'd expect it to deadlock and
crash almost immediately.

Also, it might be worthwhile creating accessor functions for the
mptctl variables or using atomics, that way the locking doesn't need
to be right every time they're used.

Finally, what's the exact race condition here? Are the functions you
reference changing the values of the mptctl variables while other code
is using them? These functions appear to be setup functions, so why
are those variables being used before the device is fully set up?
Single usages of those variables shouldn't be subject to race
conditions, so you shouldn't need mutexes around those.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 2/8] regulator: axp20x: add support for set_ramp_delay for AXP209

2018-12-11 Thread Julian Calaby
Hi Priit and Olliver,

On Tue, Dec 11, 2018 at 5:42 AM Priit Laes  wrote:
>
> From: Olliver Schinagl 
>
> The AXP209 supports ramping up voltages on several regulators such as
> DCDC2 and LDO3.
>
> This patch adds preliminary support for the regulator-ramp-delay property
> for these 2 regulators. Note that the voltage ramp only works when
> regulator is already enabled. E.g. when going from say 0.7 V to 3.6 V.
>
> When turning on the regulator, no voltage ramp is performed in hardware.
>
> What this means, is that if the bootloader brings up the voltage at 0.7 V,
> the ramp delay property is properly applied. If however, the bootloader
> leaves the power off, no ramp delay is applied when the power is
> enabled by the regulator framework.
>
> Signed-off-by: Olliver Schinagl 
> Signed-off-by: Priit Laes 
> ---
>  drivers/regulator/axp20x-regulator.c | 85 +-
>  1 file changed, 85 insertions(+)
>
> diff --git a/drivers/regulator/axp20x-regulator.c 
> b/drivers/regulator/axp20x-regulator.c
> index 9a2db28..1d9fa62 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -346,6 +357,79 @@
> .ops= _ops_range,  
>   \
> }
>
> +static const int axp209_dcdc2_ldo3_slew_rates[] = {
> +   1600,
> +800,
> +};
> +
> +static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> +{
> +   struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
> +   const struct regulator_desc *desc = rdev->desc;
> +   u8 reg, mask, enable, cfg = 0xff;
> +   const int *slew_rates;
> +   int rate_count = 0;
> +
> +   if (!rdev)
> +   return -EINVAL;
> +
> +   switch (axp20x->variant) {
> +   case AXP209_ID:
> +   if (desc->id == AXP20X_DCDC2) {

Is slew_rates supposed to be set here?

> +   rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
> +   reg = AXP20X_DCDC2_LDO3_V_RAMP;
> +   mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK |
> +  AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK;
> +   enable = (ramp > 0) ?
> +AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN :
> +!AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN;
> +   break;
> +   }
> +
> +   if (desc->id == AXP20X_LDO3) {
> +   slew_rates = axp209_dcdc2_ldo3_slew_rates;
> +   rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
> +   reg = AXP20X_DCDC2_LDO3_V_RAMP;
> +   mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK |
> +  AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK;
> +   enable = (ramp > 0) ?
> +AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN :
> +!AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN;
> +   break;
> +   }
> +
> +   if (rate_count > 0)
> +   break;

You could save one to two tests by combining the above three if statements, i.e.

if (DCDC2) {
// set DCDC2 stuff

break;
} else if (LDO3) {
// set LDO3 stuff

break;
}

As written, the rate_count > 0 test will never be true as every block
that sets rate_count breaks out of the switch block.

You could then calculate rate_count below the switch block.

> +
> +   /* fall through */
> +   default:
> +   /* Not supported for this regulator */
> +   return -ENOTSUPP;
> +   }
> +
> +   if (ramp == 0) {
> +   cfg = enable;
> +   } else {
> +   int i;
> +
> +   for (i = 0; i < rate_count; i++) {
> +   if (ramp <= slew_rates[i])
> +   cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i);
> +   else
> +   break;
> +   }
> +
> +       if (cfg == 0xff) {
> +   dev_err(axp20x->dev, "unsupported ramp value %d", 
> ramp);
> +   return -EINVAL;
> +   }
> +
> +   cfg |= enable;
> +   }
> +
> +   return regmap_update_bits(axp20x->regmap, reg, mask, cfg);
> +}
> +



-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 10/15] drm/sun4i: Add support for R40 TV TCONs

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 11:57 AM, Julian Calaby <julian.cal...@gmail.com> wrote:
> Hi Jernej,
>
> On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skra...@siol.net> 
> wrote:
>> R40 display pipeline has a lot of possible configurations. HDMI can be
>> connected to 2 different TCONs (out of 4) and mixers can be connected to
>> any TCON. All this must be configured in TCON TOP.
>>
>> Along with definition of TCON capabilities also add mux callback, which
>> can configure this complex pipeline.
>>
>> For now, only TCON TV is supported.
>>
>> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
>> ---
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index e0c562ce1c22..81b9551e4f78 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon,
>> return 0;
>>  }
>>
>> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
>> +const struct drm_encoder *encoder,
>> +int index)
>> +{
>> +   if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
>> +   sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index);
>> +
>> +   sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id,
>> +tcon_type_tv, index);
>> +
>> +   return 0;
>> +}
>> +
>> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon,
>> +  const struct drm_encoder *encoder)
>> +{
>> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0);
>> +}
>> +
>> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon,
>> +  const struct drm_encoder *encoder)
>> +{
>> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1);
>> +}
>
> Are TCON-TOPs going to be a common thing in new SoCs from Allwinner?
> If so, maybe we should add an index to the TCON quirks and have a
> common TCON-TOP set_mux function.

Actually, that only moves it up a level. Should it be a devicetree property?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 10/15] drm/sun4i: Add support for R40 TV TCONs

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 11:57 AM, Julian Calaby  wrote:
> Hi Jernej,
>
> On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec  
> wrote:
>> R40 display pipeline has a lot of possible configurations. HDMI can be
>> connected to 2 different TCONs (out of 4) and mixers can be connected to
>> any TCON. All this must be configured in TCON TOP.
>>
>> Along with definition of TCON capabilities also add mux callback, which
>> can configure this complex pipeline.
>>
>> For now, only TCON TV is supported.
>>
>> Signed-off-by: Jernej Skrabec 
>> ---
>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> index e0c562ce1c22..81b9551e4f78 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon,
>> return 0;
>>  }
>>
>> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
>> +const struct drm_encoder *encoder,
>> +int index)
>> +{
>> +   if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
>> +   sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index);
>> +
>> +   sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id,
>> +tcon_type_tv, index);
>> +
>> +   return 0;
>> +}
>> +
>> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon,
>> +  const struct drm_encoder *encoder)
>> +{
>> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0);
>> +}
>> +
>> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon,
>> +  const struct drm_encoder *encoder)
>> +{
>> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1);
>> +}
>
> Are TCON-TOPs going to be a common thing in new SoCs from Allwinner?
> If so, maybe we should add an index to the TCON quirks and have a
> common TCON-TOP set_mux function.

Actually, that only moves it up a level. Should it be a devicetree property?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 10/15] drm/sun4i: Add support for R40 TV TCONs

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skra...@siol.net> wrote:
> R40 display pipeline has a lot of possible configurations. HDMI can be
> connected to 2 different TCONs (out of 4) and mixers can be connected to
> any TCON. All this must be configured in TCON TOP.
>
> Along with definition of TCON capabilities also add mux callback, which
> can configure this complex pipeline.
>
> For now, only TCON TV is supported.
>
> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index e0c562ce1c22..81b9551e4f78 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon,
> return 0;
>  }
>
> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
> +const struct drm_encoder *encoder,
> +int index)
> +{
> +   if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
> +   sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index);
> +
> +   sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id,
> +tcon_type_tv, index);
> +
> +   return 0;
> +}
> +
> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon,
> +  const struct drm_encoder *encoder)
> +{
> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0);
> +}
> +
> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon,
> +  const struct drm_encoder *encoder)
> +{
> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1);
> +}

Are TCON-TOPs going to be a common thing in new SoCs from Allwinner?
If so, maybe we should add an index to the TCON quirks and have a
common TCON-TOP set_mux function.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 10/15] drm/sun4i: Add support for R40 TV TCONs

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec  wrote:
> R40 display pipeline has a lot of possible configurations. HDMI can be
> connected to 2 different TCONs (out of 4) and mixers can be connected to
> any TCON. All this must be configured in TCON TOP.
>
> Along with definition of TCON capabilities also add mux callback, which
> can configure this complex pipeline.
>
> For now, only TCON TV is supported.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 39 ++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index e0c562ce1c22..81b9551e4f78 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1274,6 +1274,31 @@ static int sun6i_tcon_set_mux(struct sun4i_tcon *tcon,
> return 0;
>  }
>
> +static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
> +const struct drm_encoder *encoder,
> +int index)
> +{
> +   if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
> +   sun8i_tcon_top_set_hdmi_src(tcon->tcon_top, index);
> +
> +   sun8i_tcon_top_de_config(tcon->tcon_top, tcon->id,
> +tcon_type_tv, index);
> +
> +   return 0;
> +}
> +
> +static int sun8i_r40_tcon_tv_set_mux_0(struct sun4i_tcon *tcon,
> +  const struct drm_encoder *encoder)
> +{
> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 0);
> +}
> +
> +static int sun8i_r40_tcon_tv_set_mux_1(struct sun4i_tcon *tcon,
> +  const struct drm_encoder *encoder)
> +{
> +   return sun8i_r40_tcon_tv_set_mux(tcon, encoder, 1);
> +}

Are TCON-TOPs going to be a common thing in new SoCs from Allwinner?
If so, maybe we should add an index to the TCON quirks and have a
common TCON-TOP set_mux function.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 07/15] dt-bindings: display: sun4i-drm: Add R40 HDMI pipeline

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec <jernej.skra...@siol.net> wrote:
> Missing compatibles and descriptions needed to implement R40 HDMI
> pipeline are added.
>
> For mixers only compatibles are added.
>
> TCON description is expanded with R40 TV TCON compatibles. If the SoC
> has TCON TOP unit, phandle to that unit has to be specified. Additional
> clock has to be specified if SoC has TCON TOP and TCON is TV TCON.
>
> New compatible is added for DWC HDMI PHY, which has additional clock
> specified.

There's a bunch of A64 related stuff mixed in here, is the R40
compatible with the A64's parts? If so, you should probably mention
that in the commit log.

> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
> ---
>  .../bindings/display/sunxi/sun4i-drm.txt | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index a099957ab62a..634276f713e8 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -111,8 +112,9 @@ Required properties:
>- resets: phandle to the reset controller driving the PHY
>- reset-names: must be "phy"
>
> -H3 HDMI PHY requires additional clock:
> +H3 and A64 HDMI PHY requires additional clocks:
>- pll-0: parent of phy clock
> +  - pll-1: second possible phy clock parent (A64 only)

Maybe split this into two:

H3 HDMI PHY ...
   - pll-0: ...

A64 HDMI PHY ...
   - pll-0: ...
   - pll-1: ...

At the moment a quick reading implies that H3 needs pll-1.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 07/15] dt-bindings: display: sun4i-drm: Add R40 HDMI pipeline

2018-05-19 Thread Julian Calaby
Hi Jernej,

On Sun, May 20, 2018 at 4:31 AM, Jernej Skrabec  wrote:
> Missing compatibles and descriptions needed to implement R40 HDMI
> pipeline are added.
>
> For mixers only compatibles are added.
>
> TCON description is expanded with R40 TV TCON compatibles. If the SoC
> has TCON TOP unit, phandle to that unit has to be specified. Additional
> clock has to be specified if SoC has TCON TOP and TCON is TV TCON.
>
> New compatible is added for DWC HDMI PHY, which has additional clock
> specified.

There's a bunch of A64 related stuff mixed in here, is the R40
compatible with the A64's parts? If so, you should probably mention
that in the commit log.

> Signed-off-by: Jernej Skrabec 
> ---
>  .../bindings/display/sunxi/sun4i-drm.txt | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index a099957ab62a..634276f713e8 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -111,8 +112,9 @@ Required properties:
>- resets: phandle to the reset controller driving the PHY
>- reset-names: must be "phy"
>
> -H3 HDMI PHY requires additional clock:
> +H3 and A64 HDMI PHY requires additional clocks:
>- pll-0: parent of phy clock
> +  - pll-1: second possible phy clock parent (A64 only)

Maybe split this into two:

H3 HDMI PHY ...
   - pll-0: ...

A64 HDMI PHY ...
   - pll-0: ...
   - pll-1: ...

At the moment a quick reading implies that H3 needs pll-1.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v2 1/6] ASoC: sun4i-i2s: Add slot width override

2018-03-13 Thread Julian Calaby
Hi Marcus,

On Tue, Mar 13, 2018 at 2:57 AM,  <codekip...@gmail.com> wrote:
> From: Marcus Cooper <codekip...@gmail.com>
>
> Some codecs require a different amount of a bit clocks per frame
> than what is calculated by using the sample width. Use a slot
> width override property to provide this mechanism.
>
> Signed-off-by: Marcus Cooper <codekip...@gmail.com>
> ---
>  Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  5 +
>  sound/soc/sunxi/sun4i-i2s.c   | 15 ---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt 
> b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index b9d50d6cdef3..48addef65b8f 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -28,6 +28,11 @@ Required properties for the following compatibles:
> - "allwinner,sun8i-h3-i2s"
>  - resets: phandle to the reset line for this codec
>
> +Optional properties:
> +- allwinner,slot-width-override:if this property is present then the dai is
> +   configured to extend the slot width to the
> +   value specified. Min 8, Max 32.
> +

This sounds like something that would be useful for other I2S controllers.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v2 1/6] ASoC: sun4i-i2s: Add slot width override

2018-03-13 Thread Julian Calaby
Hi Marcus,

On Tue, Mar 13, 2018 at 2:57 AM,   wrote:
> From: Marcus Cooper 
>
> Some codecs require a different amount of a bit clocks per frame
> than what is calculated by using the sample width. Use a slot
> width override property to provide this mechanism.
>
> Signed-off-by: Marcus Cooper 
> ---
>  Documentation/devicetree/bindings/sound/sun4i-i2s.txt |  5 +
>  sound/soc/sunxi/sun4i-i2s.c   | 15 ---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt 
> b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index b9d50d6cdef3..48addef65b8f 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -28,6 +28,11 @@ Required properties for the following compatibles:
> - "allwinner,sun8i-h3-i2s"
>  - resets: phandle to the reset line for this codec
>
> +Optional properties:
> +- allwinner,slot-width-override:if this property is present then the dai is
> +   configured to extend the slot width to the
> +   value specified. Min 8, Max 32.
> +

This sounds like something that would be useful for other I2S controllers.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-27 Thread Julian Calaby
Hi Maxime,

On Tue, Feb 27, 2018 at 6:07 PM, Maxime Ripard
<maxime.rip...@bootlin.com> wrote:
> On Tue, Feb 27, 2018 at 01:29:27PM +1100, Julian Calaby wrote:
>> Hi Jernej,
>>
>> On Tue, Feb 27, 2018 at 3:27 AM, Jernej Škrabec <jernej.skra...@siol.net> 
>> wrote:
>> > Hi,
>> >
>> > Dne ponedeljek, 26. februar 2018 ob 17:21:05 CET je Icenowy Zheng 
>> > napisal(a):
>> >> 于 2018年2月27日 GMT+08:00 上午12:16:44, "Jernej Škrabec"
>> > <jernej.skra...@siol.net> 写到:
>> >> >Hi Julian,
>> >> >
>> >> >Dne nedelja, 25. februar 2018 ob 09:11:34 CET je Julian Calaby
>> >> >
>> >> >napisal(a):
>> >> >> Hi Jernej,
>> >> >>
>> >> >> On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>> >> >
>> >> ><jernej.skra...@siol.net>
>> >> >
>> >> >wrote:
>> >> >> > Enable HDMI output on all boards which have HDMI connector.
>> >> >> >
>> >> >> > Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
>> >> >> > ---
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
>> >> >> >
>> >> >> >   | 25 ++
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
>> >> >> >
>> >> >> >   | 25 ++
>> >> >
>> >> >arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
>> >> >
>> >> >> > | 25 ++
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
>> >> >> >
>> >> >> >   | 24 +
>> >> >
>> >> >arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> >> >
>> >> >> >| 25 ++ 8 files changed, 199
>> >> >
>> >> >insertions(+)
>> >> >
>> >> >> As I understand it, the H2+ is just a slightly trimmed down H3. In
>> >> >> terms of HDMI support, the difference is that the H2+ can't output
>> >> >
>> >> >4k.
>> >> >
>> >> >> If this code is compatible with the H2+, could you please add the
>> >> >> necessary bits and pieces to the h2-plus DTSs too?
>> >> >
>> >> >There are only 3 H2+ boards in kernel and none of them has HDMI
>> >> >connector, so
>> >>
>> >> BPi M2 Zero has miniHDMI connector.
>> >
>> > Ah, sorry, I missed that one. Since I don't have it (or any other board 
>> > with
>> > H2+) I'm not really comfortable including such patch. If anyone will test 
>> > it,
>> > I can include it in next revision.
>>
>> I have one of those (this is why I'm interested in this patchset) so
>> I'm willing to test, however I can't guarantee I'll get to it quickly.
>
> Then I guess the best way forward will be to keep the current patch as
> is, and you'll send a patch whenever you have the time to test it.

Fair enough, I'll do that then. =)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-27 Thread Julian Calaby
Hi Maxime,

On Tue, Feb 27, 2018 at 6:07 PM, Maxime Ripard
 wrote:
> On Tue, Feb 27, 2018 at 01:29:27PM +1100, Julian Calaby wrote:
>> Hi Jernej,
>>
>> On Tue, Feb 27, 2018 at 3:27 AM, Jernej Škrabec  
>> wrote:
>> > Hi,
>> >
>> > Dne ponedeljek, 26. februar 2018 ob 17:21:05 CET je Icenowy Zheng 
>> > napisal(a):
>> >> 于 2018年2月27日 GMT+08:00 上午12:16:44, "Jernej Škrabec"
>> >  写到:
>> >> >Hi Julian,
>> >> >
>> >> >Dne nedelja, 25. februar 2018 ob 09:11:34 CET je Julian Calaby
>> >> >
>> >> >napisal(a):
>> >> >> Hi Jernej,
>> >> >>
>> >> >> On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>> >> >
>> >> >
>> >> >
>> >> >wrote:
>> >> >> > Enable HDMI output on all boards which have HDMI connector.
>> >> >> >
>> >> >> > Signed-off-by: Jernej Skrabec 
>> >> >> > ---
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
>> >> >> >
>> >> >> >   | 25 ++
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
>> >> >> >
>> >> >> >   | 25 ++
>> >> >
>> >> >arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
>> >> >
>> >> >> > | 25 ++
>> >> >> >
>> >> >> >  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>> >> >> >  ++ arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
>> >> >> >
>> >> >> >   | 24 +
>> >> >
>> >> >arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> >> >
>> >> >> >| 25 ++ 8 files changed, 199
>> >> >
>> >> >insertions(+)
>> >> >
>> >> >> As I understand it, the H2+ is just a slightly trimmed down H3. In
>> >> >> terms of HDMI support, the difference is that the H2+ can't output
>> >> >
>> >> >4k.
>> >> >
>> >> >> If this code is compatible with the H2+, could you please add the
>> >> >> necessary bits and pieces to the h2-plus DTSs too?
>> >> >
>> >> >There are only 3 H2+ boards in kernel and none of them has HDMI
>> >> >connector, so
>> >>
>> >> BPi M2 Zero has miniHDMI connector.
>> >
>> > Ah, sorry, I missed that one. Since I don't have it (or any other board 
>> > with
>> > H2+) I'm not really comfortable including such patch. If anyone will test 
>> > it,
>> > I can include it in next revision.
>>
>> I have one of those (this is why I'm interested in this patchset) so
>> I'm willing to test, however I can't guarantee I'll get to it quickly.
>
> Then I guess the best way forward will be to keep the current patch as
> is, and you'll send a patch whenever you have the time to test it.

Fair enough, I'll do that then. =)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-26 Thread Julian Calaby
Hi Jernej,

On Tue, Feb 27, 2018 at 3:27 AM, Jernej Škrabec <jernej.skra...@siol.net> wrote:
> Hi,
>
> Dne ponedeljek, 26. februar 2018 ob 17:21:05 CET je Icenowy Zheng napisal(a):
>> 于 2018年2月27日 GMT+08:00 上午12:16:44, "Jernej Škrabec"
> <jernej.skra...@siol.net> 写到:
>> >Hi Julian,
>> >
>> >Dne nedelja, 25. februar 2018 ob 09:11:34 CET je Julian Calaby
>> >
>> >napisal(a):
>> >> Hi Jernej,
>> >>
>> >> On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>> >
>> ><jernej.skra...@siol.net>
>> >
>> >wrote:
>> >> > Enable HDMI output on all boards which have HDMI connector.
>> >> >
>> >> > Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
>> >> > ---
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
>> >> >
>> >> >   | 25 ++
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
>> >> >
>> >> >   | 25 ++
>> >
>> >arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
>> >
>> >> > | 25 ++
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
>> >> >
>> >> >   | 24 +
>> >
>> >arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> >
>> >> >| 25 ++ 8 files changed, 199
>> >
>> >insertions(+)
>> >
>> >> As I understand it, the H2+ is just a slightly trimmed down H3. In
>> >> terms of HDMI support, the difference is that the H2+ can't output
>> >
>> >4k.
>> >
>> >> If this code is compatible with the H2+, could you please add the
>> >> necessary bits and pieces to the h2-plus DTSs too?
>> >
>> >There are only 3 H2+ boards in kernel and none of them has HDMI
>> >connector, so
>>
>> BPi M2 Zero has miniHDMI connector.
>
> Ah, sorry, I missed that one. Since I don't have it (or any other board with
> H2+) I'm not really comfortable including such patch. If anyone will test it,
> I can include it in next revision.

I have one of those (this is why I'm interested in this patchset) so
I'm willing to test, however I can't guarantee I'll get to it quickly.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-26 Thread Julian Calaby
Hi Jernej,

On Tue, Feb 27, 2018 at 3:27 AM, Jernej Škrabec  wrote:
> Hi,
>
> Dne ponedeljek, 26. februar 2018 ob 17:21:05 CET je Icenowy Zheng napisal(a):
>> 于 2018年2月27日 GMT+08:00 上午12:16:44, "Jernej Škrabec"
>  写到:
>> >Hi Julian,
>> >
>> >Dne nedelja, 25. februar 2018 ob 09:11:34 CET je Julian Calaby
>> >
>> >napisal(a):
>> >> Hi Jernej,
>> >>
>> >> On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>> >
>> >
>> >
>> >wrote:
>> >> > Enable HDMI output on all boards which have HDMI connector.
>> >> >
>> >> > Signed-off-by: Jernej Skrabec 
>> >> > ---
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-beelink-x2.dts
>> >> >
>> >> >   | 25 ++
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts
>> >> >
>> >> >   | 25 ++
>> >
>> >arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
>> >
>> >> > | 25 ++
>> >> >
>> >> >  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>> >> >  ++ arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
>> >> >
>> >> >   | 24 +
>> >
>> >arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> >
>> >> >| 25 ++ 8 files changed, 199
>> >
>> >insertions(+)
>> >
>> >> As I understand it, the H2+ is just a slightly trimmed down H3. In
>> >> terms of HDMI support, the difference is that the H2+ can't output
>> >
>> >4k.
>> >
>> >> If this code is compatible with the H2+, could you please add the
>> >> necessary bits and pieces to the h2-plus DTSs too?
>> >
>> >There are only 3 H2+ boards in kernel and none of them has HDMI
>> >connector, so
>>
>> BPi M2 Zero has miniHDMI connector.
>
> Ah, sorry, I missed that one. Since I don't have it (or any other board with
> H2+) I'm not really comfortable including such patch. If anyone will test it,
> I can include it in next revision.

I have one of those (this is why I'm interested in this patchset) so
I'm willing to test, however I can't guarantee I'll get to it quickly.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-25 Thread Julian Calaby
Hi Icenowy,

On Sun, Feb 25, 2018 at 7:43 PM, Icenowy Zheng <icen...@aosc.io> wrote:
>
>
> 于 2018年2月25日 GMT+08:00 下午4:11:34, Julian Calaby <julian.cal...@gmail.com> 写到:
>>Hi Jernej,
>>
>>On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>><jernej.skra...@siol.net> wrote:
>>> Enable HDMI output on all boards which have HDMI connector.
>>>
>>> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
>>> ---
>>>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-one.dts| 24
>>+
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 25
>>++
>>>  8 files changed, 199 insertions(+)
>>
>>As I understand it, the H2+ is just a slightly trimmed down H3. In
>>terms of HDMI support, the difference is that the H2+ can't output 4k.
>
> H2+ can OUTPUT 4K. The BSP restricts it to DECODE 4K. (And mainline won't 
> have such restriction)

Interesting!

I'm getting my data from here: http://linux-sunxi.org/H3#Variants

So essentially what you're saying is that the H2+ is just a H3 with
less pins? (I'm assuming the "lack of gigabit mac" is effectively it
not having the required pins, i.e. GPIO bank D)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-25 Thread Julian Calaby
Hi Icenowy,

On Sun, Feb 25, 2018 at 7:43 PM, Icenowy Zheng  wrote:
>
>
> 于 2018年2月25日 GMT+08:00 下午4:11:34, Julian Calaby  写到:
>>Hi Jernej,
>>
>>On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec
>> wrote:
>>> Enable HDMI output on all boards which have HDMI connector.
>>>
>>> Signed-off-by: Jernej Skrabec 
>>> ---
>>>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25
>>++
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-one.dts| 24
>>+
>>>  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 25
>>++
>>>  8 files changed, 199 insertions(+)
>>
>>As I understand it, the H2+ is just a slightly trimmed down H3. In
>>terms of HDMI support, the difference is that the H2+ can't output 4k.
>
> H2+ can OUTPUT 4K. The BSP restricts it to DECODE 4K. (And mainline won't 
> have such restriction)

Interesting!

I'm getting my data from here: http://linux-sunxi.org/H3#Variants

So essentially what you're saying is that the H2+ is just a H3 with
less pins? (I'm assuming the "lack of gigabit mac" is effectively it
not having the required pins, i.e. GPIO bank D)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-25 Thread Julian Calaby
Hi Jernej,

On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec <jernej.skra...@siol.net> wrote:
> Enable HDMI output on all boards which have HDMI connector.
>
> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
> ---
>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-one.dts| 24 +
>  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 25 
> ++
>  8 files changed, 199 insertions(+)

As I understand it, the H2+ is just a slightly trimmed down H3. In
terms of HDMI support, the difference is that the H2+ can't output 4k.
If this code is compatible with the H2+, could you please add the
necessary bits and pieces to the h2-plus DTSs too?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 14/15] ARM: dts: sun8i: h3: Enable HDMI output on H3 boards

2018-02-25 Thread Julian Calaby
Hi Jernej,

On Sun, Feb 25, 2018 at 8:45 AM, Jernej Skrabec  wrote:
> Enable HDMI output on all boards which have HDMI connector.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts| 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-lite.dts   | 25 
> ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-one.dts| 24 +
>  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 25 
> ++
>  8 files changed, 199 insertions(+)

As I understand it, the H2+ is just a slightly trimmed down H3. In
terms of HDMI support, the difference is that the H2+ can't output 4k.
If this code is compatible with the H2+, could you please add the
necessary bits and pieces to the h2-plus DTSs too?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v3] rtc: ac100: Fix ac100 determine rate bug

2018-02-15 Thread Julian Calaby
Hi Phillipp,

On Thu, Feb 15, 2018 at 12:56 AM, Philipp Rossak <embe...@gmail.com> wrote:
> This patch fixes a bug, that prevents the Allwinner A83T and the A80
> from a successful boot.
>
> The bug is there since v4.16-rc1 and appeared after the clk branch was
> merged.
>
> You can find the shortend trace below:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 
> pgd = (ptrval)
> [] *pgd=
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 49 Comm: kworker/0:1 Not tainted 4.15.0-10190-gb89e32ccd1be #2
> Hardware name: Allwinner sun8i Family
> Workqueue: events deferred_probe_work_func
> PC is at clk_hw_get_rate+0x0/0x34
> LR is at ac100_clkout_determine_rate+0x48/0x19c
>
> [ ... ]
>
> (clk_hw_get_rate) from (ac100_clkout_determine_rate+0x48/0x19c)
> (ac100_clkout_determine_rate) from  (clk_core_set_rate_nolock+0x3c/0x1a0)
> (clk_core_set_rate_nolock) from (clk_set_rate+0x30/0x88)
> (clk_set_rate) from (of_clk_set_defaults+0x200/0x364)
> (of_clk_set_defaults) from (platform_drv_probe+0x18/0xb0)
>
> To fix that bug, we first check if the return of the
> clk_hw_get_parent_by_index is non zero. If it is zero we skip that
> clock parent.
>
> The BUG report could be found here: https://lkml.org/lkml/2018/2/10/198
>
> Fixes: 04940631b8d2 ("rtc: ac100: Add clk output support")
>
> Signed-off-by: Philipp Rossak <embe...@gmail.com>
> ---
>
> Changes in v3:
> * add information when the bug appeared
> * make the comment more clear
> Changes in v2:
> * add tag Fixes: ... to commit message
> * add comment to if statement why we are doing this check
>
>  drivers/rtc/rtc-ac100.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
> index 8ff9dc3fe5bf..2412aa2e8399 100644
> --- a/drivers/rtc/rtc-ac100.c
> +++ b/drivers/rtc/rtc-ac100.c
> @@ -183,7 +183,24 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
>
> for (i = 0; i < num_parents; i++) {
> struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> -   unsigned long tmp, prate = clk_hw_get_rate(parent);
> +   unsigned long tmp, prate;
> +
> +   /*
> +* The clock has two parents, one is a fixed clock which is
> +* internally registered by the ac100 driver. The other parent
> +* is a clock from the codec side of the chip, which we
> +* properly declare and reference in the devicetree and is
> +* not implemented in any driver right now.
> +* If the clock core looks for the parent of that second
> +* missing clock, it can't one that is registered and

You've missed the word "find" between "it can't" and "one that is registered".

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v3] rtc: ac100: Fix ac100 determine rate bug

2018-02-15 Thread Julian Calaby
Hi Phillipp,

On Thu, Feb 15, 2018 at 12:56 AM, Philipp Rossak  wrote:
> This patch fixes a bug, that prevents the Allwinner A83T and the A80
> from a successful boot.
>
> The bug is there since v4.16-rc1 and appeared after the clk branch was
> merged.
>
> You can find the shortend trace below:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 
> pgd = (ptrval)
> [] *pgd=
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 49 Comm: kworker/0:1 Not tainted 4.15.0-10190-gb89e32ccd1be #2
> Hardware name: Allwinner sun8i Family
> Workqueue: events deferred_probe_work_func
> PC is at clk_hw_get_rate+0x0/0x34
> LR is at ac100_clkout_determine_rate+0x48/0x19c
>
> [ ... ]
>
> (clk_hw_get_rate) from (ac100_clkout_determine_rate+0x48/0x19c)
> (ac100_clkout_determine_rate) from  (clk_core_set_rate_nolock+0x3c/0x1a0)
> (clk_core_set_rate_nolock) from (clk_set_rate+0x30/0x88)
> (clk_set_rate) from (of_clk_set_defaults+0x200/0x364)
> (of_clk_set_defaults) from (platform_drv_probe+0x18/0xb0)
>
> To fix that bug, we first check if the return of the
> clk_hw_get_parent_by_index is non zero. If it is zero we skip that
> clock parent.
>
> The BUG report could be found here: https://lkml.org/lkml/2018/2/10/198
>
> Fixes: 04940631b8d2 ("rtc: ac100: Add clk output support")
>
> Signed-off-by: Philipp Rossak 
> ---
>
> Changes in v3:
> * add information when the bug appeared
> * make the comment more clear
> Changes in v2:
> * add tag Fixes: ... to commit message
> * add comment to if statement why we are doing this check
>
>  drivers/rtc/rtc-ac100.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-ac100.c b/drivers/rtc/rtc-ac100.c
> index 8ff9dc3fe5bf..2412aa2e8399 100644
> --- a/drivers/rtc/rtc-ac100.c
> +++ b/drivers/rtc/rtc-ac100.c
> @@ -183,7 +183,24 @@ static int ac100_clkout_determine_rate(struct clk_hw *hw,
>
> for (i = 0; i < num_parents; i++) {
> struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> -   unsigned long tmp, prate = clk_hw_get_rate(parent);
> +   unsigned long tmp, prate;
> +
> +   /*
> +* The clock has two parents, one is a fixed clock which is
> +* internally registered by the ac100 driver. The other parent
> +* is a clock from the codec side of the chip, which we
> +* properly declare and reference in the devicetree and is
> +* not implemented in any driver right now.
> +* If the clock core looks for the parent of that second
> +* missing clock, it can't one that is registered and

You've missed the word "find" between "it can't" and "one that is registered".

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 1/3] net: stmmac: dwmac-sun8i: drop V3s compatible and add V3 one

2018-02-02 Thread Julian Calaby
Hi Icenowy,

On Sat, Feb 3, 2018 at 5:04 AM, Icenowy Zheng <icen...@aosc.io> wrote:
> The V3s is just a differently packaged version of the V3 chip, which has
> a MAC with the same capability with H3. The V3s just doesn't wire out
> the external MII/RMII/RGMII bus. (V3 wired out it).
>
> Drop the compatible string of V3s in the dwmac-sun8i driver, and add a
> V3 compatible string, which has all capabilities.

Aren't compatible strings technically API, so don't we need to support
those that are out in the wild "forever"?

Therefore shouldn't we leave the v3s variant around for compatibility
with existing device trees?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 1/3] net: stmmac: dwmac-sun8i: drop V3s compatible and add V3 one

2018-02-02 Thread Julian Calaby
Hi Icenowy,

On Sat, Feb 3, 2018 at 5:04 AM, Icenowy Zheng  wrote:
> The V3s is just a differently packaged version of the V3 chip, which has
> a MAC with the same capability with H3. The V3s just doesn't wire out
> the external MII/RMII/RGMII bus. (V3 wired out it).
>
> Drop the compatible string of V3s in the dwmac-sun8i driver, and add a
> V3 compatible string, which has all capabilities.

Aren't compatible strings technically API, so don't we need to support
those that are out in the wild "forever"?

Therefore shouldn't we leave the v3s variant around for compatibility
with existing device trees?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 2/9] ARM: sunxi: add Allwinner ARMv5 SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 2:10 PM, Icenowy Zheng <icen...@aosc.io> wrote:
>
>
> 于 2018年1月20日 GMT+08:00 上午11:06:40, Julian Calaby <julian.cal...@gmail.com> 写到:
>>Hi Icenowy,
>>
>>On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng <icen...@aosc.io>
>>wrote:
>>> Add option for Allwinner ARMv5 SoCs and a SoC suniv (which is a die
>>used
>>> for many new F-series products, including F1C100A, F1C100s, F1C200s,
>>> F1C500, F1C600).
>>>
>>> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
>>> ---
>>>  arch/arm/mach-sunxi/Kconfig| 13 +
>>>  arch/arm/mach-sunxi/Makefile   |  1 +
>>>  arch/arm/mach-sunxi/sunxi_v5.c | 22 ++
>>>  3 files changed, 36 insertions(+)
>>>  create mode 100644 arch/arm/mach-sunxi/sunxi_v5.c
>>>
>>> diff --git a/arch/arm/mach-sunxi/Kconfig
>>b/arch/arm/mach-sunxi/Kconfig
>>> index 65509a35935f..78ac9ce70641 100644
>>> --- a/arch/arm/mach-sunxi/Kconfig
>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>> @@ -59,3 +59,16 @@ config MACH_SUN9I
>>> select ARM_GIC
>>>
>>>  endif
>>> +
>>> +menuconfig ARCH_SUNXI_V5
>>> +   bool "Allwinner SoCs"
>>
>>That name seems a little too generic. Maybe "Allwinner ARMv5 SoCs"?
>
> This is already required by armv5.
>
> Allwinner currently has only ARMv5,7,8 SoCs. ARMv8 is under
> arm64 architecture, and ARMv5 and v7 cannot be selected at the same time.

I'm going to try to back my way out of this hole by saying that they
should be more descriptive anyway (and it'll give clueless kconfiggers
a hint as to why they're not seeing their SoC listed)

However you're right, if both cannot be visible at the same time, then
it really doesn't matter if they both have the same name.

Sorry for the noise,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 2/9] ARM: sunxi: add Allwinner ARMv5 SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 2:10 PM, Icenowy Zheng  wrote:
>
>
> 于 2018年1月20日 GMT+08:00 上午11:06:40, Julian Calaby  写到:
>>Hi Icenowy,
>>
>>On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng 
>>wrote:
>>> Add option for Allwinner ARMv5 SoCs and a SoC suniv (which is a die
>>used
>>> for many new F-series products, including F1C100A, F1C100s, F1C200s,
>>> F1C500, F1C600).
>>>
>>> Signed-off-by: Icenowy Zheng 
>>> ---
>>>  arch/arm/mach-sunxi/Kconfig| 13 +
>>>  arch/arm/mach-sunxi/Makefile   |  1 +
>>>  arch/arm/mach-sunxi/sunxi_v5.c | 22 ++
>>>  3 files changed, 36 insertions(+)
>>>  create mode 100644 arch/arm/mach-sunxi/sunxi_v5.c
>>>
>>> diff --git a/arch/arm/mach-sunxi/Kconfig
>>b/arch/arm/mach-sunxi/Kconfig
>>> index 65509a35935f..78ac9ce70641 100644
>>> --- a/arch/arm/mach-sunxi/Kconfig
>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>> @@ -59,3 +59,16 @@ config MACH_SUN9I
>>> select ARM_GIC
>>>
>>>  endif
>>> +
>>> +menuconfig ARCH_SUNXI_V5
>>> +   bool "Allwinner SoCs"
>>
>>That name seems a little too generic. Maybe "Allwinner ARMv5 SoCs"?
>
> This is already required by armv5.
>
> Allwinner currently has only ARMv5,7,8 SoCs. ARMv8 is under
> arm64 architecture, and ARMv5 and v7 cannot be selected at the same time.

I'm going to try to back my way out of this hole by saying that they
should be more descriptive anyway (and it'll give clueless kconfiggers
a hint as to why they're not seeing their SoC listed)

However you're right, if both cannot be visible at the same time, then
it really doesn't matter if they both have the same name.

Sorry for the noise,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 2/9] ARM: sunxi: add Allwinner ARMv5 SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng <icen...@aosc.io> wrote:
> Add option for Allwinner ARMv5 SoCs and a SoC suniv (which is a die used
> for many new F-series products, including F1C100A, F1C100s, F1C200s,
> F1C500, F1C600).
>
> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
> ---
>  arch/arm/mach-sunxi/Kconfig| 13 +
>  arch/arm/mach-sunxi/Makefile   |  1 +
>  arch/arm/mach-sunxi/sunxi_v5.c | 22 ++
>  3 files changed, 36 insertions(+)
>  create mode 100644 arch/arm/mach-sunxi/sunxi_v5.c
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 65509a35935f..78ac9ce70641 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -59,3 +59,16 @@ config MACH_SUN9I
> select ARM_GIC
>
>  endif
> +
> +menuconfig ARCH_SUNXI_V5
> +   bool "Allwinner SoCs"

That name seems a little too generic. Maybe "Allwinner ARMv5 SoCs"?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 2/9] ARM: sunxi: add Allwinner ARMv5 SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng  wrote:
> Add option for Allwinner ARMv5 SoCs and a SoC suniv (which is a die used
> for many new F-series products, including F1C100A, F1C100s, F1C200s,
> F1C500, F1C600).
>
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/mach-sunxi/Kconfig| 13 +
>  arch/arm/mach-sunxi/Makefile   |  1 +
>  arch/arm/mach-sunxi/sunxi_v5.c | 22 ++
>  3 files changed, 36 insertions(+)
>  create mode 100644 arch/arm/mach-sunxi/sunxi_v5.c
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 65509a35935f..78ac9ce70641 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -59,3 +59,16 @@ config MACH_SUN9I
> select ARM_GIC
>
>  endif
> +
> +menuconfig ARCH_SUNXI_V5
> +   bool "Allwinner SoCs"

That name seems a little too generic. Maybe "Allwinner ARMv5 SoCs"?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 1/9] ARM: add CONFIG_ARCH_SUNXI_V7 for differentiate ARMv5/v7 Allwinner SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng <icen...@aosc.io> wrote:
> Allwinner also has some ARMv5 SoCs.
>
> In order to add support for them, add a CONFIG_ARCH_SUNXI_V7 option that
> is selectable when ARMv7 is selceted, and make CONFIG_ARCH_SUNXI a
> common bool config which is selected by both V7 and V5 sunxi option.
>
> The ARMv7 defconfigs are modified to have the new CONFIG_ARCH_SUNXI_V7
> option.
>
> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
> ---
>  arch/arm/configs/multi_v7_defconfig |  2 +-
>  arch/arm/configs/sunxi_defconfig|  2 +-
>  arch/arm/mach-sunxi/Kconfig | 14 --
>  arch/arm/mach-sunxi/Makefile|  2 +-
>  4 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 58153cdf025b..65509a35935f 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1,6 +1,16 @@
> -menuconfig ARCH_SUNXI
> +config ARCH_SUNXI
> +   bool
> +   select ARCH_HAS_RESET_CONTROLLER
> +   select CLKSRC_MMIO
> +   select GENERIC_IRQ_CHIP
> +   select GPIOLIB
> +   select PINCTRL
> +   select RESET_CONTROLLER
> +
> +menuconfig ARCH_SUNXI_V7
> bool "Allwinner SoCs"
> depends on ARCH_MULTI_V7
> +   select ARCH_SUNXI
> select ARCH_HAS_RESET_CONTROLLER
> select CLKSRC_MMIO
> select GENERIC_IRQ_CHIP

Shouldn't you remove all the common ARCH_SUNXI selects from ARCH_SUNXI_v7?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [RFC PATCH 1/9] ARM: add CONFIG_ARCH_SUNXI_V7 for differentiate ARMv5/v7 Allwinner SoCs

2018-01-19 Thread Julian Calaby
Hi Icenowy,

On Sat, Jan 20, 2018 at 10:17 AM, Icenowy Zheng  wrote:
> Allwinner also has some ARMv5 SoCs.
>
> In order to add support for them, add a CONFIG_ARCH_SUNXI_V7 option that
> is selectable when ARMv7 is selceted, and make CONFIG_ARCH_SUNXI a
> common bool config which is selected by both V7 and V5 sunxi option.
>
> The ARMv7 defconfigs are modified to have the new CONFIG_ARCH_SUNXI_V7
> option.
>
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/configs/multi_v7_defconfig |  2 +-
>  arch/arm/configs/sunxi_defconfig|  2 +-
>  arch/arm/mach-sunxi/Kconfig | 14 --
>  arch/arm/mach-sunxi/Makefile|  2 +-
>  4 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 58153cdf025b..65509a35935f 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1,6 +1,16 @@
> -menuconfig ARCH_SUNXI
> +config ARCH_SUNXI
> +   bool
> +   select ARCH_HAS_RESET_CONTROLLER
> +   select CLKSRC_MMIO
> +   select GENERIC_IRQ_CHIP
> +   select GPIOLIB
> +   select PINCTRL
> +   select RESET_CONTROLLER
> +
> +menuconfig ARCH_SUNXI_V7
> bool "Allwinner SoCs"
> depends on ARCH_MULTI_V7
> +   select ARCH_SUNXI
> select ARCH_HAS_RESET_CONTROLLER
> select CLKSRC_MMIO
>     select GENERIC_IRQ_CHIP

Shouldn't you remove all the common ARCH_SUNXI selects from ARCH_SUNXI_v7?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v2 13/16] power: supply: axp20x_battery: add support for AXP813

2018-01-09 Thread Julian Calaby
Hi Quentin,

On Tue, Jan 9, 2018 at 8:33 PM, Quentin Schulz
<quentin.sch...@free-electrons.com> wrote:
> The X-Powers AXP813 PMIC has got some slight differences from
> AXP20X/AXP22X PMICs:
>  - the maximum voltage supplied by the PMIC is 4.35 instead of 4.36/4.24
>  for AXP20X/AXP22X,
>  - the constant charge current formula is different,
>
> It also has a bit to tell whether the battery percentage returned by the
> PMIC is valid.
>
> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
> ---
>  drivers/power/supply/axp20x_battery.c | 42 -
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/power/supply/axp20x_battery.c 
> b/drivers/power/supply/axp20x_battery.c
> index d73c78f..dad72a5 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -46,6 +46,8 @@
>  #define AXP20X_CHRG_CTRL1_TGT_4_2V (2 << 5)
>  #define AXP20X_CHRG_CTRL1_TGT_4_36V(3 << 5)
>
> +#define AXP813_CHRG_CTRL1_TGT_4_35V(3 << 5)
> +
>  #define AXP22X_CHRG_CTRL1_TGT_4_22V(1 << 5)
>  #define AXP22X_CHRG_CTRL1_TGT_4_24V(3 << 5)

Should these be "alphabetical", i.e. AXP20X, AXP22X, AXP813?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v2 13/16] power: supply: axp20x_battery: add support for AXP813

2018-01-09 Thread Julian Calaby
Hi Quentin,

On Tue, Jan 9, 2018 at 8:33 PM, Quentin Schulz
 wrote:
> The X-Powers AXP813 PMIC has got some slight differences from
> AXP20X/AXP22X PMICs:
>  - the maximum voltage supplied by the PMIC is 4.35 instead of 4.36/4.24
>  for AXP20X/AXP22X,
>  - the constant charge current formula is different,
>
> It also has a bit to tell whether the battery percentage returned by the
> PMIC is valid.
>
> Signed-off-by: Quentin Schulz 
> ---
>  drivers/power/supply/axp20x_battery.c | 42 -
>  1 file changed, 42 insertions(+)
>
> diff --git a/drivers/power/supply/axp20x_battery.c 
> b/drivers/power/supply/axp20x_battery.c
> index d73c78f..dad72a5 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -46,6 +46,8 @@
>  #define AXP20X_CHRG_CTRL1_TGT_4_2V (2 << 5)
>  #define AXP20X_CHRG_CTRL1_TGT_4_36V(3 << 5)
>
> +#define AXP813_CHRG_CTRL1_TGT_4_35V(3 << 5)
> +
>  #define AXP22X_CHRG_CTRL1_TGT_4_22V(1 << 5)
>  #define AXP22X_CHRG_CTRL1_TGT_4_24V(3 << 5)

Should these be "alphabetical", i.e. AXP20X, AXP22X, AXP813?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 16/67] powerpc: rename dma_direct_ to dma_nommu_

2018-01-03 Thread Julian Calaby
Hi All,

On Wed, Jan 3, 2018 at 6:49 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote:
> Hi Michael,
>
> On Wed, Jan 3, 2018 at 7:24 AM, Michael Ellerman <m...@ellerman.id.au> wrote:
>> Geert Uytterhoeven <ge...@linux-m68k.org> writes:
>>
>>> On Tue, Jan 2, 2018 at 10:45 AM, Michael Ellerman <m...@ellerman.id.au> 
>>> wrote:
>>>> Christoph Hellwig <h...@lst.de> writes:
>>>>
>>>>> We want to use the dma_direct_ namespace for a generic implementation,
>>>>> so rename powerpc to the second best choice: dma_nommu_.
>>>>
>>>> I'm not a fan of "nommu". Some of the users of direct ops *are* using an
>>>> IOMMU, they're just setting up a 1:1 mapping once at init time, rather
>>>> than mapping dynamically.
>>>>
>>>> Though I don't have a good idea for a better name, maybe "1to1",
>>>> "linear", "premapped" ?
>>>
>>> "identity"?
>>
>> I think that would be wrong, but thanks for trying to help :)
>>
>> The address on the device side is sometimes (often?) offset from the CPU
>> address. So eg. the device can DMA to RAM address 0x0 using address
>> 0x800.
>>
>> Identity would imply 0 == 0 etc.
>>
>> I think "bijective" is the correct term, but that's probably a bit
>> esoteric.
>
> OK, didn't know about the offset.
> Then "linear" is what we tend to use, right?

If this is indeed a linear mapping, can we just remove this and
replace it with the new "generic" mapping being introduced by this
patchset?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 16/67] powerpc: rename dma_direct_ to dma_nommu_

2018-01-03 Thread Julian Calaby
Hi All,

On Wed, Jan 3, 2018 at 6:49 PM, Geert Uytterhoeven  wrote:
> Hi Michael,
>
> On Wed, Jan 3, 2018 at 7:24 AM, Michael Ellerman  wrote:
>> Geert Uytterhoeven  writes:
>>
>>> On Tue, Jan 2, 2018 at 10:45 AM, Michael Ellerman  
>>> wrote:
>>>> Christoph Hellwig  writes:
>>>>
>>>>> We want to use the dma_direct_ namespace for a generic implementation,
>>>>> so rename powerpc to the second best choice: dma_nommu_.
>>>>
>>>> I'm not a fan of "nommu". Some of the users of direct ops *are* using an
>>>> IOMMU, they're just setting up a 1:1 mapping once at init time, rather
>>>> than mapping dynamically.
>>>>
>>>> Though I don't have a good idea for a better name, maybe "1to1",
>>>> "linear", "premapped" ?
>>>
>>> "identity"?
>>
>> I think that would be wrong, but thanks for trying to help :)
>>
>> The address on the device side is sometimes (often?) offset from the CPU
>> address. So eg. the device can DMA to RAM address 0x0 using address
>> 0x800.
>>
>> Identity would imply 0 == 0 etc.
>>
>> I think "bijective" is the correct term, but that's probably a bit
>> esoteric.
>
> OK, didn't know about the offset.
> Then "linear" is what we tend to use, right?

If this is indeed a linear mapping, can we just remove this and
replace it with the new "generic" mapping being introduced by this
patchset?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 17/67] microblaze: rename dma_direct to dma_microblaze

2017-12-29 Thread Julian Calaby
Hi Christoph,

On Fri, Dec 29, 2017 at 7:18 PM, Christoph Hellwig <h...@lst.de> wrote:
> This frees the dma_direct_* namespace for a generic implementation.

Don't you mean "dma_nommu" not "dma_microblaze" in the subject line?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 17/67] microblaze: rename dma_direct to dma_microblaze

2017-12-29 Thread Julian Calaby
Hi Christoph,

On Fri, Dec 29, 2017 at 7:18 PM, Christoph Hellwig  wrote:
> This frees the dma_direct_* namespace for a generic implementation.

Don't you mean "dma_nommu" not "dma_microblaze" in the subject line?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 11/17] drm/sun4i: Wire in DE2 scaler support

2017-11-29 Thread Julian Calaby
Hi Jernej,

On Tue, Nov 28, 2017 at 7:57 AM, Jernej Skrabec <jernej.skra...@siol.net> wrote:
> Calculate scaling parameters and call appropriate scaler set up
> function.
>
> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
> ---
>  drivers/gpu/drm/sun4i/sun8i_layer.c |  12 +++-
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 115 
> +---
>  drivers/gpu/drm/sun4i/sun8i_mixer.h |   4 --
>  3 files changed, 90 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_layer.c
> index e1b6ad82145e..6860271e5415 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_layer.c
> @@ -41,9 +44,14 @@ static int sun8i_mixer_layer_atomic_check(struct drm_plane 
> *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> +   scaler_supported = !!(layer->mixer->cfg->scaler_mask & 
> BIT(layer->id));
> +
> +   min_scale = scaler_supported ? 1 : DRM_PLANE_HELPER_NO_SCALING;
> +   max_scale = scaler_supported ? (1UL << 20) - 1 :
> +  DRM_PLANE_HELPER_NO_SCALING;
> +

Disclaimer: I hate ternaries, but this looks like it'd be better
written as an if statement. I.e.

min_scale = DRM_PLANE_HELPER_NO_SCALING;
max_scale = DRM_PLANE_HELPER_NO_SCALING;

if (layer->mixer->cfg->scaler_mask & BIT(layer->id)) {
min_scale = 1;
max_scale = (1UL << 20) - 1;
}

However the compiler will probably sort it all out anyway, so it
probably doesn't matter that much, except for style.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 11/17] drm/sun4i: Wire in DE2 scaler support

2017-11-29 Thread Julian Calaby
Hi Jernej,

On Tue, Nov 28, 2017 at 7:57 AM, Jernej Skrabec  wrote:
> Calculate scaling parameters and call appropriate scaler set up
> function.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun8i_layer.c |  12 +++-
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 115 
> +---
>  drivers/gpu/drm/sun4i/sun8i_mixer.h |   4 --
>  3 files changed, 90 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_layer.c
> index e1b6ad82145e..6860271e5415 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_layer.c
> @@ -41,9 +44,14 @@ static int sun8i_mixer_layer_atomic_check(struct drm_plane 
> *plane,
> clip.x2 = crtc_state->adjusted_mode.hdisplay;
> clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> +   scaler_supported = !!(layer->mixer->cfg->scaler_mask & 
> BIT(layer->id));
> +
> +   min_scale = scaler_supported ? 1 : DRM_PLANE_HELPER_NO_SCALING;
> +   max_scale = scaler_supported ? (1UL << 20) - 1 :
> +  DRM_PLANE_HELPER_NO_SCALING;
> +

Disclaimer: I hate ternaries, but this looks like it'd be better
written as an if statement. I.e.

min_scale = DRM_PLANE_HELPER_NO_SCALING;
max_scale = DRM_PLANE_HELPER_NO_SCALING;

if (layer->mixer->cfg->scaler_mask & BIT(layer->id)) {
min_scale = 1;
max_scale = (1UL << 20) - 1;
}

However the compiler will probably sort it all out anyway, so it
probably doesn't matter that much, except for style.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 5/5] wlcore: Use common error handling code in wl1271_op_suspend()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:16 AM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 20:36:39 +0100
>
> Add a jump target so that a specific error message is stored only once
> at the end of this function implementation.
> Replace two calls of the macro "wl1271_warning" by goto statements.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

This patch is bogus, moving error messages around like this is just bizarre.

These are both reporting different failures, so the error messages
should be different.

> ---
>  drivers/net/wireless/ti/wlcore/main.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 12a9d6509382..a110f61110d5 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -1732,8 +1732,7 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
> ret = wl1271_configure_suspend(wl, wlvif, wow);
> if (ret < 0) {
> mutex_unlock(>mutex);
> -   wl1271_warning("couldn't prepare device to suspend");

"couldn't configure device for suspend"?

> -   return ret;
> +   goto report_preparation_failure;
> }
> }
>
> @@ -1752,10 +1751,8 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
> wl1271_ps_elp_sleep(wl);
> mutex_unlock(>mutex);
>
> -   if (ret < 0) {
> -   wl1271_warning("couldn't prepare device to suspend");

"couldn't enable power saving"?

> -   return ret;
> -   }
> +   if (ret < 0)
> +   goto report_preparation_failure;
>
> /* flush any remaining work */
> wl1271_debug(DEBUG_MAC80211, "flushing remaining works");

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 5/5] wlcore: Use common error handling code in wl1271_op_suspend()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:16 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sun, 29 Oct 2017 20:36:39 +0100
>
> Add a jump target so that a specific error message is stored only once
> at the end of this function implementation.
> Replace two calls of the macro "wl1271_warning" by goto statements.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 

This patch is bogus, moving error messages around like this is just bizarre.

These are both reporting different failures, so the error messages
should be different.

> ---
>  drivers/net/wireless/ti/wlcore/main.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 12a9d6509382..a110f61110d5 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -1732,8 +1732,7 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
> ret = wl1271_configure_suspend(wl, wlvif, wow);
> if (ret < 0) {
> mutex_unlock(>mutex);
> -   wl1271_warning("couldn't prepare device to suspend");

"couldn't configure device for suspend"?

> -   return ret;
> +   goto report_preparation_failure;
> }
> }
>
> @@ -1752,10 +1751,8 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
> wl1271_ps_elp_sleep(wl);
> mutex_unlock(>mutex);
>
> -   if (ret < 0) {
> -   wl1271_warning("couldn't prepare device to suspend");

"couldn't enable power saving"?

> -   return ret;
> -   }
> +   if (ret < 0)
> +   goto report_preparation_failure;
>
> /* flush any remaining work */
> wl1271_debug(DEBUG_MAC80211, "flushing remaining works");

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 4/5] wlcore: Use common error handling code in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:14 AM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 20:16:22 +0100
>
> Adjust jump targets so that a bit of exception handling can be better
> reused at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

However,

> ---
>  drivers/net/wireless/ti/wlcore/main.c | 18 +++---
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 0365b5e40a8d..12a9d6509382 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -4081,9 +4078,8 @@ static int wlcore_set_beacon_template(struct wl1271 *wl,
>   beacon->data,
>   beacon->len, 0,
>       min_rate);
> -end_bcn:
> +free_skb:

Why rename the label?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 4/5] wlcore: Use common error handling code in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:14 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sun, 29 Oct 2017 20:16:22 +0100
>
> Adjust jump targets so that a bit of exception handling can be better
> reused at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 

Reviewed-by: Julian Calaby 

However,

> ---
>  drivers/net/wireless/ti/wlcore/main.c | 18 +++---
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 0365b5e40a8d..12a9d6509382 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -4081,9 +4078,8 @@ static int wlcore_set_beacon_template(struct wl1271 *wl,
>   beacon->data,
>   beacon->len, 0,
>   min_rate);
> -end_bcn:
> +free_skb:

Why rename the label?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/5] wlcore: Return directly after a failed ieee80211_beacon_get() in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
On Mon, Oct 30, 2017 at 7:13 AM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 20:00:41 +0100
>
> Return directly after a call of the function "ieee80211_beacon_get"
> failed at the beginning.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/5] wlcore: Return directly after a failed ieee80211_beacon_get() in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
On Mon, Oct 30, 2017 at 7:13 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sun, 29 Oct 2017 20:00:41 +0100
>
> Return directly after a call of the function "ieee80211_beacon_get"
> failed at the beginning.
>
> Signed-off-by: Markus Elfring 

Reviewed-by: Julian Calaby 

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 1/5] wlcore: Use common error handling code in wlcore_nvs_cb()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:11 AM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 18:38:04 +0100
>
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
>  drivers/net/wireless/ti/wlcore/main.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index c346c021b999..48380d48ef09 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -6551,6 +6549,11 @@ static void wlcore_nvs_cb(const struct firmware *fw, 
> void *context)
>  out:
> release_firmware(fw);
> complete_all(>nvs_loading_complete);
> +   return;
> +
> +power_off:

Name this "out_power_off" to match the other labels.

> +   wl1271_power_off(wl);
> +   goto out_free_nvs;

Why not put this in front of the out_free_nvs label? It looks weird here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 1/5] wlcore: Use common error handling code in wlcore_nvs_cb()

2017-10-30 Thread Julian Calaby
Hi Markus,

On Mon, Oct 30, 2017 at 7:11 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sun, 29 Oct 2017 18:38:04 +0100
>
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/net/wireless/ti/wlcore/main.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index c346c021b999..48380d48ef09 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -6551,6 +6549,11 @@ static void wlcore_nvs_cb(const struct firmware *fw, 
> void *context)
>  out:
> release_firmware(fw);
> complete_all(>nvs_loading_complete);
> +   return;
> +
> +power_off:

Name this "out_power_off" to match the other labels.

> +   wl1271_power_off(wl);
> +   goto out_free_nvs;

Why not put this in front of the out_free_nvs label? It looks weird here.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 2/5] wlcore: Delete an unnecessary check statement in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
On Mon, Oct 30, 2017 at 7:12 AM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 19:45:07 +0100
>
> A goto statement jumped to a target which followed a condition check
> immediately without the specification of useful actions between.
> Thus remove such unnecessary source code at the end of this function.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Looks good to me.

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 2/5] wlcore: Delete an unnecessary check statement in wlcore_set_beacon_template()

2017-10-30 Thread Julian Calaby
On Mon, Oct 30, 2017 at 7:12 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Sun, 29 Oct 2017 19:45:07 +0100
>
> A goto statement jumped to a target which followed a condition check
> immediately without the specification of useful actions between.
> Thus remove such unnecessary source code at the end of this function.
>
> Signed-off-by: Markus Elfring 

Looks good to me.

Reviewed-by: Julian Calaby 

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v4 02/11] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-10-11 Thread Julian Calaby
Hi Chen-Yu,

On Tue, Oct 10, 2017 at 2:19 PM, Chen-Yu Tsai <w...@csie.org> wrote:
> On systems with 2 TCONs such as the A31, it is possible to demux the
> output of the TCONs to one encoder.
>
> Add support for this for the A31.
>
> Signed-off-by: Chen-Yu Tsai <w...@csie.org>

Thanks!

FWIW this is:

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH v4 02/11] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-10-11 Thread Julian Calaby
Hi Chen-Yu,

On Tue, Oct 10, 2017 at 2:19 PM, Chen-Yu Tsai  wrote:
> On systems with 2 TCONs such as the A31, it is possible to demux the
> output of the TCONs to one encoder.
>
> Add support for this for the A31.
>
> Signed-off-by: Chen-Yu Tsai 

Thanks!

FWIW this is:

Reviewed-by: Julian Calaby 

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-30 Thread Julian Calaby
Hi Chen-Yu,

On Sat, Sep 30, 2017 at 3:58 PM, Chen-Yu Tsai <w...@csie.org> wrote:
> On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby <julian.cal...@gmail.com> 
> wrote:
>> Hi Chen-Yu,
>>
>> On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <w...@csie.org> wrote:
>>> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard
>>> <maxime.rip...@free-electrons.com> wrote:
>>>> On Fri, Sep 29, 2017 at 08:22:56AM +, Chen-Yu Tsai wrote:
>>>>> On systems with 2 TCONs such as the A31, it is possible to demux the
>>>>> output of the TCONs to one encoder.
>>>>>
>>>>> Add support for this for the A31.
>>>>>
>>>>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>>>>> ---
>>>>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 
>>>>> ++
>>>>>  1 file changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>>>>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> index 7bf51abaee97..c949309d4285 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon 
>>>>> *tcon, bool enable)
>>>>>  }
>>>>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>>>>
>>>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>>>>
>>>> Would that make sense to make it a bit more generic, and pass the id
>>>> to look for as an argument?
>>>
>>> The reason to look for TCON0 explicitly is to access the muxing registers, 
>>> which
>>> are only available in TCON0. Other than that, there's nothing else
>>> shared between
>>> the two TCONs. So there's no particular reason to look for TCON1 explicitly.
>>
>> In that case: in the bizarre case where we're trying to use this mux
>> type and there is no TCON0, shouldn't we fail?
>
> It gives out a big warning, indicating something is wrong. If TCON0 is not 
> found
> it is most likely your device tree is broken. There's nothing more the
> driver can do.
> Are you suggesting to return NULL in this case, and also do error
> handling in the
> callers?

You're already returning -EINVAL for other failure cases, so a lack of
TCON0 might as well do the same.

>> (Also, the code doesn't make sense if we have some TCON1 and TCON2 in
>> that order as it'll return TCON2)
>
> I'm guessing you want it to return NULL.

I'm just pointing out the mismatch between getting the "first" TCON
and the actual behaviour.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-30 Thread Julian Calaby
Hi Chen-Yu,

On Sat, Sep 30, 2017 at 3:58 PM, Chen-Yu Tsai  wrote:
> On Sat, Sep 30, 2017 at 1:35 PM, Julian Calaby  
> wrote:
>> Hi Chen-Yu,
>>
>> On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai  wrote:
>>> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard
>>>  wrote:
>>>> On Fri, Sep 29, 2017 at 08:22:56AM +, Chen-Yu Tsai wrote:
>>>>> On systems with 2 TCONs such as the A31, it is possible to demux the
>>>>> output of the TCONs to one encoder.
>>>>>
>>>>> Add support for this for the A31.
>>>>>
>>>>> Signed-off-by: Chen-Yu Tsai 
>>>>> ---
>>>>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 
>>>>> ++
>>>>>  1 file changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>>>>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> index 7bf51abaee97..c949309d4285 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon 
>>>>> *tcon, bool enable)
>>>>>  }
>>>>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>>>>
>>>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>>>>
>>>> Would that make sense to make it a bit more generic, and pass the id
>>>> to look for as an argument?
>>>
>>> The reason to look for TCON0 explicitly is to access the muxing registers, 
>>> which
>>> are only available in TCON0. Other than that, there's nothing else
>>> shared between
>>> the two TCONs. So there's no particular reason to look for TCON1 explicitly.
>>
>> In that case: in the bizarre case where we're trying to use this mux
>> type and there is no TCON0, shouldn't we fail?
>
> It gives out a big warning, indicating something is wrong. If TCON0 is not 
> found
> it is most likely your device tree is broken. There's nothing more the
> driver can do.
> Are you suggesting to return NULL in this case, and also do error
> handling in the
> callers?

You're already returning -EINVAL for other failure cases, so a lack of
TCON0 might as well do the same.

>> (Also, the code doesn't make sense if we have some TCON1 and TCON2 in
>> that order as it'll return TCON2)
>
> I'm guessing you want it to return NULL.

I'm just pointing out the mismatch between getting the "first" TCON
and the actual behaviour.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-29 Thread Julian Calaby
Hi Chen-Yu,

On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai <w...@csie.org> wrote:
> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard
> <maxime.rip...@free-electrons.com> wrote:
>> On Fri, Sep 29, 2017 at 08:22:56AM +, Chen-Yu Tsai wrote:
>>> On systems with 2 TCONs such as the A31, it is possible to demux the
>>> output of the TCONs to one encoder.
>>>
>>> Add support for this for the A31.
>>>
>>> Signed-off-by: Chen-Yu Tsai <w...@csie.org>
>>> ---
>>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 
>>> ++
>>>  1 file changed, 38 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> index 7bf51abaee97..c949309d4285 100644
>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, 
>>> bool enable)
>>>  }
>>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>>
>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>>
>> Would that make sense to make it a bit more generic, and pass the id
>> to look for as an argument?
>
> The reason to look for TCON0 explicitly is to access the muxing registers, 
> which
> are only available in TCON0. Other than that, there's nothing else
> shared between
> the two TCONs. So there's no particular reason to look for TCON1 explicitly.

In that case: in the bizarre case where we're trying to use this mux
type and there is no TCON0, shouldn't we fail?

(Also, the code doesn't make sense if we have some TCON1 and TCON2 in
that order as it'll return TCON2)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] Re: [PATCH v3 04/14] drm/sun4i: tcon: Add support for demuxing TCON output on A31

2017-09-29 Thread Julian Calaby
Hi Chen-Yu,

On Fri, Sep 29, 2017 at 8:22 PM, Chen-Yu Tsai  wrote:
> On Fri, Sep 29, 2017 at 6:20 PM, Maxime Ripard
>  wrote:
>> On Fri, Sep 29, 2017 at 08:22:56AM +, Chen-Yu Tsai wrote:
>>> On systems with 2 TCONs such as the A31, it is possible to demux the
>>> output of the TCONs to one encoder.
>>>
>>> Add support for this for the A31.
>>>
>>> Signed-off-by: Chen-Yu Tsai 
>>> ---
>>>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 38 
>>> ++
>>>  1 file changed, 38 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
>>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> index 7bf51abaee97..c949309d4285 100644
>>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
>>> @@ -112,6 +112,21 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, 
>>> bool enable)
>>>  }
>>>  EXPORT_SYMBOL(sun4i_tcon_enable_vblank);
>>>
>>> +static struct sun4i_tcon *sun4i_get_first_tcon(struct drm_device *drm)
>>
>> Would that make sense to make it a bit more generic, and pass the id
>> to look for as an argument?
>
> The reason to look for TCON0 explicitly is to access the muxing registers, 
> which
> are only available in TCON0. Other than that, there's nothing else
> shared between
> the two TCONs. So there's no particular reason to look for TCON1 explicitly.

In that case: in the bizarre case where we're trying to use this mux
type and there is no TCON0, shouldn't we fail?

(Also, the code doesn't make sense if we have some TCON1 and TCON2 in
that order as it'll return TCON2)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 3/4] arm64: allwinner: h5: add compatible string for DE2 CCU

2017-09-16 Thread Julian Calaby
Hi Icenowy,

On Tue, Sep 12, 2017 at 1:55 AM, Icenowy Zheng <icen...@aosc.io> wrote:
> The DE2 CCU on Allwinner H5 SoC has a slightly different behavior than
> the one on H3, so the compatible string is not set in the common DTSI
> file.
>
> Add the compatible string of H5 DE2 CCU in H5 DTSI file.
>
> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> index d9a720bff05d..e237c05cfdb4 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> @@ -98,6 +98,10 @@
> compatible = "allwinner,sun50i-h5-ccu";
>  };
>
> +_clocks {
> +   compatible = "allwinner,sun50i-h5-de2-clk";
> +};
> +

This is what I get for reviewing before reading the full patch set.

Shouldn't this be rolled into the previous patch?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 3/4] arm64: allwinner: h5: add compatible string for DE2 CCU

2017-09-16 Thread Julian Calaby
Hi Icenowy,

On Tue, Sep 12, 2017 at 1:55 AM, Icenowy Zheng  wrote:
> The DE2 CCU on Allwinner H5 SoC has a slightly different behavior than
> the one on H3, so the compatible string is not set in the common DTSI
> file.
>
> Add the compatible string of H5 DE2 CCU in H5 DTSI file.
>
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> index d9a720bff05d..e237c05cfdb4 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
> @@ -98,6 +98,10 @@
> compatible = "allwinner,sun50i-h5-ccu";
>  };
>
> +_clocks {
> +   compatible = "allwinner,sun50i-h5-de2-clk";
> +};
> +

This is what I get for reviewing before reading the full patch set.

Shouldn't this be rolled into the previous patch?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 2/4] ARM: sun8i: h3/h5: add DE2 CCU device node for H3

2017-09-16 Thread Julian Calaby
Hi Icenowy,

On Tue, Sep 12, 2017 at 1:55 AM, Icenowy Zheng <icen...@aosc.io> wrote:
> The DE2 in H3/H5 has a clock control unit in it, and the behavior is
> slightly different between H3 and H5.
>
> Add the common parts in H3/H5 DTSI, and add the compatible string in H3
> DTSI.
>
> The compatible string of H5 DE2 CCU will be added in a separated patch.
>
> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi|  4 
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 14 ++
>  2 files changed, 18 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 11240a8313c2..76a4cbc99bdb 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -85,6 +87,18 @@
> #size-cells = <1>;
> ranges;
>
> +   display_clocks: clock@100 {
> +   /* compatible is in per SoC .dtsi file */

I don't know device tree very well, but shouldn't this node be
disabled so that it doesn't do anything weird on H5? Or are nodes
without compatibles ignored?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 2/4] ARM: sun8i: h3/h5: add DE2 CCU device node for H3

2017-09-16 Thread Julian Calaby
Hi Icenowy,

On Tue, Sep 12, 2017 at 1:55 AM, Icenowy Zheng  wrote:
> The DE2 in H3/H5 has a clock control unit in it, and the behavior is
> slightly different between H3 and H5.
>
> Add the common parts in H3/H5 DTSI, and add the compatible string in H3
> DTSI.
>
> The compatible string of H5 DE2 CCU will be added in a separated patch.
>
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi|  4 
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 14 ++
>  2 files changed, 18 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 11240a8313c2..76a4cbc99bdb 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -85,6 +87,18 @@
> #size-cells = <1>;
> ranges;
>
> +   display_clocks: clock@100 {
> +   /* compatible is in per SoC .dtsi file */

I don't know device tree very well, but shouldn't this node be
disabled so that it doesn't do anything weird on H5? Or are nodes
without compatibles ignored?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [v6] wlcore: add missing nvs file name info for wilink8

2017-08-10 Thread Julian Calaby
Hi Eyal,

On Thu, Aug 10, 2017 at 4:35 PM, Reizer, Eyal <ey...@ti.com> wrote:
>> >
>> > Sorry for top posting (mobile...)
>> > I have verified with system design and the data sheet that every wilink 6/7
>> chip has a mac address in fuse so probably the board you have (pretty old,
>> right?) has this mac address in fuse. Maybe it was from very early batches?
>> Anyway I see no reason to change it.
>> > Anyway the calibrator can be used to store a different one into the nvs 
>> > file
>> that will overide it.
>>
>> Well clearly at least this one does not have any valid hardware
>> mac address, the hardware mac address is broken with all zeroes.
>>
>
> Looks like it is not really all zeros but rather 00:00:00:00:00:01.
> I wonder if it is just a one board issue or not...

It's 00:00:00:00:00:01 because your code adds 1 to it.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [v6] wlcore: add missing nvs file name info for wilink8

2017-08-10 Thread Julian Calaby
Hi Eyal,

On Thu, Aug 10, 2017 at 4:35 PM, Reizer, Eyal  wrote:
>> >
>> > Sorry for top posting (mobile...)
>> > I have verified with system design and the data sheet that every wilink 6/7
>> chip has a mac address in fuse so probably the board you have (pretty old,
>> right?) has this mac address in fuse. Maybe it was from very early batches?
>> Anyway I see no reason to change it.
>> > Anyway the calibrator can be used to store a different one into the nvs 
>> > file
>> that will overide it.
>>
>> Well clearly at least this one does not have any valid hardware
>> mac address, the hardware mac address is broken with all zeroes.
>>
>
> Looks like it is not really all zeros but rather 00:00:00:00:00:01.
> I wonder if it is just a one board issue or not...

It's 00:00:00:00:00:01 because your code adds 1 to it.

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [v4] wlcore: add missing nvs file name info for wilink8

2017-08-04 Thread Julian Calaby
Hi Eyal,

On Mon, Jul 31, 2017 at 6:45 PM, Reizer, Eyal <ey...@ti.com> wrote:
> The following commits:
> c815fde wlcore: spi: Populate config firmware data
> d776fc8 wlcore: sdio: Populate config firmware data
>
> Populated the nvs entry for wilink6 and wilink7 only while it is
> still needed for wilink8 as well.
> This broke user space backward compatibility when upgrading from older
> kernels, as the alternate mac address would not be read from the nvs that
> is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> causing mac address change of the wlan interface.
>
> This patch fix this and update the structure field with the same default
> nvs file name that has been used before.
>
> In addition, some distros hold a default wl1271-nvs.bin in the file
> system with a bogus mac address (deadbeef...) that for a wl18xx device
> also overrides the mac address that is stored inside the device.
> Warn users about this bogus mac address and use a random mac instead
>
> Cc: stable <sta...@vger.kernel.org>
> Signed-off-by: Eyal Reizer <ey...@ti.com>
> ---
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 60aaa85..7ce4221 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -5961,6 +5961,22 @@ static void wl12xx_derive_mac_addresses(struct wl1271 
> *wl, u32 oui, u32 nic)
> if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr > 0xff)
> wl1271_warning("NIC part of the MAC address wraps around!");
>
> +   if (oui == 0xdeadbe && nic == 0xef) {
> +   wl1271_warning("Detected unconfigured mac address in nvs.\n"
> +   "Using a random TI mac address instead.\n"
> +   "in case of using a wl12xx device, your "
> +   "device performance may not be optimized.\n"
> +   "Please use the calibrator tool to configure "
> +   "your device.\n"
> +   "When using a wl18xx device the nvs file can "
> +   "be removed as a default mac address is "
> +   "stored internally.\n");
> +
> +   /* Use TI oui and a random nic */
> +   oui = 0x080028;

Is there (or should there be) a constant for this?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [v4] wlcore: add missing nvs file name info for wilink8

2017-08-04 Thread Julian Calaby
Hi Eyal,

On Mon, Jul 31, 2017 at 6:45 PM, Reizer, Eyal  wrote:
> The following commits:
> c815fde wlcore: spi: Populate config firmware data
> d776fc8 wlcore: sdio: Populate config firmware data
>
> Populated the nvs entry for wilink6 and wilink7 only while it is
> still needed for wilink8 as well.
> This broke user space backward compatibility when upgrading from older
> kernels, as the alternate mac address would not be read from the nvs that
> is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> causing mac address change of the wlan interface.
>
> This patch fix this and update the structure field with the same default
> nvs file name that has been used before.
>
> In addition, some distros hold a default wl1271-nvs.bin in the file
> system with a bogus mac address (deadbeef...) that for a wl18xx device
> also overrides the mac address that is stored inside the device.
> Warn users about this bogus mac address and use a random mac instead
>
> Cc: stable 
> Signed-off-by: Eyal Reizer 
> ---
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c 
> b/drivers/net/wireless/ti/wlcore/main.c
> index 60aaa85..7ce4221 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -5961,6 +5961,22 @@ static void wl12xx_derive_mac_addresses(struct wl1271 
> *wl, u32 oui, u32 nic)
> if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr > 0xff)
> wl1271_warning("NIC part of the MAC address wraps around!");
>
> +   if (oui == 0xdeadbe && nic == 0xef) {
> +   wl1271_warning("Detected unconfigured mac address in nvs.\n"
> +   "Using a random TI mac address instead.\n"
> +   "in case of using a wl12xx device, your "
> +   "device performance may not be optimized.\n"
> +   "Please use the calibrator tool to configure "
> +   "your device.\n"
> +   "When using a wl18xx device the nvs file can "
> +   "be removed as a default mac address is "
> +   "stored internally.\n");
> +
> +   /* Use TI oui and a random nic */
> +   oui = 0x080028;

Is there (or should there be) a constant for this?

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] sparc64: Fix gup_huge_pmd

2017-06-22 Thread Julian Calaby
Hi Nitin,

On Fri, Jun 23, 2017 at 12:37 AM, Nitin Gupta <nitin.m.gu...@oracle.com> wrote:
> Hi Julian,
>
>
> On 6/22/17 3:53 AM, Julian Calaby wrote:
>>
>> On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gu...@oracle.com>
>> wrote:
>>>
>>> The function assumes that each PMD points to head of a
>>> huge page. This is not correct as a PMD can point to
>>> start of any 8M region with a, say 256M, hugepage. The
>>> fix ensures that it points to the correct head of any PMD
>>> huge page.
>>>
>>> Signed-off-by: Nitin Gupta <nitin.m.gu...@oracle.com>
>>> ---
>>>   arch/sparc/mm/gup.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
>>> index cd0e32b..9116a6f 100644
>>> --- a/arch/sparc/mm/gup.c
>>> +++ b/arch/sparc/mm/gup.c
>>> @@ -80,6 +80,8 @@ static int gup_huge_pmd(pmd_t *pmdp, pmd_t pmd,
>>> unsigned long addr,
>>>  refs = 0;
>>>  head = pmd_page(pmd);
>>>  page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
>>> +   if (PageTail(head))
>>> +   head = compound_head(head);
>>
>> Stupid question: shouldn't this go before the page calculation?
>
>
> No, it should be after page calculation: First, 'head' points to base of
> the PMD page, then 'page' points to an offset within that page. Finally,
> we make sure that head variable points to head of the compound page
> which contains the addr.

Thanks for the explanation, that makes a bit more sense to me.

> I think confusion comes from the use of 'head' for pointing to a
> non-head page. So, maybe it would be more clear to write that part
> of the function this way:
>
> page = pmd_page(pmd) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> head = compound_head(page);

More verbose variable names would help too. =)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] sparc64: Fix gup_huge_pmd

2017-06-22 Thread Julian Calaby
Hi Nitin,

On Fri, Jun 23, 2017 at 12:37 AM, Nitin Gupta  wrote:
> Hi Julian,
>
>
> On 6/22/17 3:53 AM, Julian Calaby wrote:
>>
>> On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta 
>> wrote:
>>>
>>> The function assumes that each PMD points to head of a
>>> huge page. This is not correct as a PMD can point to
>>> start of any 8M region with a, say 256M, hugepage. The
>>> fix ensures that it points to the correct head of any PMD
>>> huge page.
>>>
>>> Signed-off-by: Nitin Gupta 
>>> ---
>>>   arch/sparc/mm/gup.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
>>> index cd0e32b..9116a6f 100644
>>> --- a/arch/sparc/mm/gup.c
>>> +++ b/arch/sparc/mm/gup.c
>>> @@ -80,6 +80,8 @@ static int gup_huge_pmd(pmd_t *pmdp, pmd_t pmd,
>>> unsigned long addr,
>>>  refs = 0;
>>>  head = pmd_page(pmd);
>>>  page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
>>> +   if (PageTail(head))
>>> +   head = compound_head(head);
>>
>> Stupid question: shouldn't this go before the page calculation?
>
>
> No, it should be after page calculation: First, 'head' points to base of
> the PMD page, then 'page' points to an offset within that page. Finally,
> we make sure that head variable points to head of the compound page
> which contains the addr.

Thanks for the explanation, that makes a bit more sense to me.

> I think confusion comes from the use of 'head' for pointing to a
> non-head page. So, maybe it would be more clear to write that part
> of the function this way:
>
> page = pmd_page(pmd) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> head = compound_head(page);

More verbose variable names would help too. =)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] sparc64: Fix gup_huge_pmd

2017-06-22 Thread Julian Calaby
Hi Nitin,

On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta <nitin.m.gu...@oracle.com> wrote:
> The function assumes that each PMD points to head of a
> huge page. This is not correct as a PMD can point to
> start of any 8M region with a, say 256M, hugepage. The
> fix ensures that it points to the correct head of any PMD
> huge page.
>
> Signed-off-by: Nitin Gupta <nitin.m.gu...@oracle.com>
> ---
>  arch/sparc/mm/gup.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
> index cd0e32b..9116a6f 100644
> --- a/arch/sparc/mm/gup.c
> +++ b/arch/sparc/mm/gup.c
> @@ -80,6 +80,8 @@ static int gup_huge_pmd(pmd_t *pmdp, pmd_t pmd, unsigned 
> long addr,
> refs = 0;
> head = pmd_page(pmd);
> page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> +   if (PageTail(head))
> +   head = compound_head(head);

Stupid question: shouldn't this go before the page calculation?

> do {
>     VM_BUG_ON(compound_head(page) != head);
> pages[*nr] = page;

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] sparc64: Fix gup_huge_pmd

2017-06-22 Thread Julian Calaby
Hi Nitin,

On Thu, Jun 22, 2017 at 7:50 AM, Nitin Gupta  wrote:
> The function assumes that each PMD points to head of a
> huge page. This is not correct as a PMD can point to
> start of any 8M region with a, say 256M, hugepage. The
> fix ensures that it points to the correct head of any PMD
> huge page.
>
> Signed-off-by: Nitin Gupta 
> ---
>  arch/sparc/mm/gup.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
> index cd0e32b..9116a6f 100644
> --- a/arch/sparc/mm/gup.c
> +++ b/arch/sparc/mm/gup.c
> @@ -80,6 +80,8 @@ static int gup_huge_pmd(pmd_t *pmdp, pmd_t pmd, unsigned 
> long addr,
> refs = 0;
> head = pmd_page(pmd);
> page = head + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> +   if (PageTail(head))
> +   head = compound_head(head);

Stupid question: shouldn't this go before the page calculation?

> do {
> VM_BUG_ON(compound_head(page) != head);
> pages[*nr] = page;

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 28/44] sparc: remove arch specific dma_supported implementations

2017-06-08 Thread Julian Calaby
Hi Christoph,

On Thu, Jun 8, 2017 at 11:25 PM, Christoph Hellwig <h...@lst.de> wrote:
> Usually dma_supported decisions are done by the dma_map_ops instance.
> Switch sparc to that model by providing a ->dma_supported instance for
> sbus that always returns false, and implementations tailored to the sun4u
> and sun4v cases for sparc64, and leave it unimplemented for PCI on
> sparc32, which means always supported.
>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
>  arch/sparc/include/asm/dma-mapping.h |  3 ---
>  arch/sparc/kernel/iommu.c| 40 
> +++-
>  arch/sparc/kernel/ioport.c   | 22 ++--
>  arch/sparc/kernel/pci_sun4v.c| 17 +++
>  4 files changed, 39 insertions(+), 43 deletions(-)
>
> diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
> index dd081d557609..12894f259bea 100644
> --- a/arch/sparc/kernel/ioport.c
> +++ b/arch/sparc/kernel/ioport.c
> @@ -401,6 +401,11 @@ static void sbus_sync_sg_for_device(struct device *dev, 
> struct scatterlist *sg,
> BUG();
>  }
>
> +static int sbus_dma_supported(struct device *dev, u64 mask)
> +{
> +   return 0;
> +}
> +

I'm guessing there's a few places that have DMA ops but DMA isn't
actually supported. Why not have a common method for this, maybe
"dma_not_supported"?

>  static const struct dma_map_ops sbus_dma_ops = {
> .alloc  = sbus_alloc_coherent,
> .free   = sbus_free_coherent,
> @@ -410,6 +415,7 @@ static const struct dma_map_ops sbus_dma_ops = {
> .unmap_sg   = sbus_unmap_sg,
> .sync_sg_for_cpu= sbus_sync_sg_for_cpu,
> .sync_sg_for_device = sbus_sync_sg_for_device,
> +   .dma_supported  = sbus_dma_supported,
>  };
>
>  static int __init sparc_register_ioport(void)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 28/44] sparc: remove arch specific dma_supported implementations

2017-06-08 Thread Julian Calaby
Hi Christoph,

On Thu, Jun 8, 2017 at 11:25 PM, Christoph Hellwig  wrote:
> Usually dma_supported decisions are done by the dma_map_ops instance.
> Switch sparc to that model by providing a ->dma_supported instance for
> sbus that always returns false, and implementations tailored to the sun4u
> and sun4v cases for sparc64, and leave it unimplemented for PCI on
> sparc32, which means always supported.
>
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/sparc/include/asm/dma-mapping.h |  3 ---
>  arch/sparc/kernel/iommu.c| 40 
> +++-
>  arch/sparc/kernel/ioport.c   | 22 ++--
>  arch/sparc/kernel/pci_sun4v.c| 17 +++
>  4 files changed, 39 insertions(+), 43 deletions(-)
>
> diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
> index dd081d557609..12894f259bea 100644
> --- a/arch/sparc/kernel/ioport.c
> +++ b/arch/sparc/kernel/ioport.c
> @@ -401,6 +401,11 @@ static void sbus_sync_sg_for_device(struct device *dev, 
> struct scatterlist *sg,
> BUG();
>  }
>
> +static int sbus_dma_supported(struct device *dev, u64 mask)
> +{
> +   return 0;
> +}
> +

I'm guessing there's a few places that have DMA ops but DMA isn't
actually supported. Why not have a common method for this, maybe
"dma_not_supported"?

>  static const struct dma_map_ops sbus_dma_ops = {
> .alloc  = sbus_alloc_coherent,
> .free   = sbus_free_coherent,
> @@ -410,6 +415,7 @@ static const struct dma_map_ops sbus_dma_ops = {
> .unmap_sg   = sbus_unmap_sg,
> .sync_sg_for_cpu= sbus_sync_sg_for_cpu,
> .sync_sg_for_device = sbus_sync_sg_for_device,
> +   .dma_supported  = sbus_dma_supported,
>  };
>
>  static int __init sparc_register_ioport(void)

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 5/15] clk: sunxi-ng: sun5i: Export video PLLs

2017-03-07 Thread Julian Calaby
Hi Maxime,

On Tue, Mar 7, 2017 at 7:56 PM, Maxime Ripard
<maxime.rip...@free-electrons.com> wrote:
> The video PLLs are used directly by the HDMI controller. Export them so
> that we can use them in our DT node.
>
> Signed-off-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> ---
>  drivers/clk/sunxi-ng/ccu-sun5i.h  | 6 --
>  include/dt-bindings/clock/sun5i-ccu.h | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun5i.h 
> b/drivers/clk/sunxi-ng/ccu-sun5i.h
> index 8144487eb7ca..16f7aa92957e 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun5i.h
> +++ b/drivers/clk/sunxi-ng/ccu-sun5i.h
> @@ -28,15 +28,17 @@
>  #define CLK_PLL_AUDIO_4X   6
>  #define CLK_PLL_AUDIO_8X   7
>  #define CLK_PLL_VIDEO0 8
> -#define CLK_PLL_VIDEO0_2X  9
> +
> +/* The PLL_VIDEO0_2X is exported for HDMI */
> +
>  #define CLK_PLL_VE 10
>  #define CLK_PLL_DDR_BASE   11
>  #define CLK_PLL_DDR12
>  #define CLK_PLL_DDR_OTHER  13
>  #define CLK_PLL_PERIPH 14
>  #define CLK_PLL_VIDEO1 15
> -#define CLK_PLL_VIDEO1_2X  16
>
> +/* The PLL_VIDEO0_2X is exported for HDMI */

PLL_VIDEO*1*_2X, right?

>  /* The CPU clock is exported */
>
>  #define CLK_AXI18

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [linux-sunxi] [PATCH 5/15] clk: sunxi-ng: sun5i: Export video PLLs

2017-03-07 Thread Julian Calaby
Hi Maxime,

On Tue, Mar 7, 2017 at 7:56 PM, Maxime Ripard
 wrote:
> The video PLLs are used directly by the HDMI controller. Export them so
> that we can use them in our DT node.
>
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/clk/sunxi-ng/ccu-sun5i.h  | 6 --
>  include/dt-bindings/clock/sun5i-ccu.h | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu-sun5i.h 
> b/drivers/clk/sunxi-ng/ccu-sun5i.h
> index 8144487eb7ca..16f7aa92957e 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun5i.h
> +++ b/drivers/clk/sunxi-ng/ccu-sun5i.h
> @@ -28,15 +28,17 @@
>  #define CLK_PLL_AUDIO_4X   6
>  #define CLK_PLL_AUDIO_8X   7
>  #define CLK_PLL_VIDEO0 8
> -#define CLK_PLL_VIDEO0_2X  9
> +
> +/* The PLL_VIDEO0_2X is exported for HDMI */
> +
>  #define CLK_PLL_VE 10
>  #define CLK_PLL_DDR_BASE   11
>  #define CLK_PLL_DDR12
>  #define CLK_PLL_DDR_OTHER  13
>  #define CLK_PLL_PERIPH 14
>  #define CLK_PLL_VIDEO1 15
> -#define CLK_PLL_VIDEO1_2X  16
>
> +/* The PLL_VIDEO0_2X is exported for HDMI */

PLL_VIDEO*1*_2X, right?

>  /* The CPU clock is exported */
>
>  #define CLK_AXI18

Thanks,

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/3] Staging:wilc1000:host_interface: Integrated two 'if' statements to a single 'if' statement

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:38 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Removed unnecessary 'if' statement and integrated the condition to the
> previous 'if' statement.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/host_interface.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/3] Staging:wilc1000:host_interface: Integrated two 'if' statements to a single 'if' statement

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:38 AM, Georgios Emmanouil  wrote:
> Removed unnecessary 'if' statement and integrated the condition to the
> previous 'if' statement.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/host_interface.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 2/3] Staging:wilc1000:host_interface: Fixed alignment to match open parenthesis

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:37 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Fixed alignment to match open parenthesis.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/host_interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 2/3] Staging:wilc1000:host_interface: Fixed alignment to match open parenthesis

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:37 AM, Georgios Emmanouil  wrote:
> Fixed alignment to match open parenthesis.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/host_interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 1/3] Staging:wilc1000:host_interface: Removed unnecessary blank line

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:35 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Removed unnecessary blank line.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/host_interface.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index c307cce..090fd43 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -287,7 +287,6 @@ static int wilc_enqueue_cmd(struct host_if_msg *msg)
> return 0;
>  }
>
> -
>  /* The u8IfIdx starts from 0 to NUM_CONCURRENT_IFC -1, but 0 index used as
>   * special purpose in wilc device, so we add 1 to the index to starts from 1.
>   * As a result, the returned index will be 1 to NUM_CONCURRENT_IFC.
> --
> 2.1.4
>



-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 1/3] Staging:wilc1000:host_interface: Removed unnecessary blank line

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 2:35 AM, Georgios Emmanouil  wrote:
> Removed unnecessary blank line.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/host_interface.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c 
> b/drivers/staging/wilc1000/host_interface.c
> index c307cce..090fd43 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -287,7 +287,6 @@ static int wilc_enqueue_cmd(struct host_if_msg *msg)
> return 0;
>  }
>
> -
>  /* The u8IfIdx starts from 0 to NUM_CONCURRENT_IFC -1, but 0 index used as
>   * special purpose in wilc device, so we add 1 to the index to starts from 1.
>   * As a result, the returned index will be 1 to NUM_CONCURRENT_IFC.
> --
> 2.1.4
>



-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] Staging:wilc1000:wilc_sdio: Modified comment style to preferred kernel comment style

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 3:41 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Modified comment style to preferred kernel comment style.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/wilc_sdio.c | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] Staging:wilc1000:wilc_sdio: Modified comment style to preferred kernel comment style

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 3:41 AM, Georgios Emmanouil  wrote:
> Modified comment style to preferred kernel comment style.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/wilc_sdio.c | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] Staging:wilc1000:linux_wlan: Modified the 'if-else' statement

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 3:14 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Modified the 'if-else' statement to make it more readable.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Again, I'm dubious if this is needed or helpful, but

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/linux_wlan.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH] Staging:wilc1000:linux_wlan: Modified the 'if-else' statement

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 3:14 AM, Georgios Emmanouil  wrote:
> Modified the 'if-else' statement to make it more readable.
>
> Signed-off-by: Georgios Emmanouil 

Again, I'm dubious if this is needed or helpful, but

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/linux_wlan.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 4/4] Staging:wilc1000:wilc_spi: Added blank line after function and modified comment style

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 4:07 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Added blank line after function and modified comment style.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/wilc_spi.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 4/4] Staging:wilc1000:wilc_spi: Added blank line after function and modified comment style

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 4:07 AM, Georgios Emmanouil  wrote:
> Added blank line after function and modified comment style.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/wilc_spi.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/4] Staging:wilc1000:wilc_spi: Fixed spelling error

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 4:06 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Fixed spelling error. 'unkmown' to 'unknown'.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/wilc_spi.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 3/4] Staging:wilc1000:wilc_spi: Fixed spelling error

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 4:06 AM, Georgios Emmanouil  wrote:
> Fixed spelling error. 'unkmown' to 'unknown'.
>
> Signed-off-by: Georgios Emmanouil 

Reviewed-by: Julian Calaby 

> ---
>  drivers/staging/wilc1000/wilc_spi.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


Re: [PATCH 2/4] Staging:wilc1000:wilc_spi: Fixed alignment to match parenthesis

2017-03-04 Thread Julian Calaby
Hi All,

On Fri, Mar 3, 2017 at 4:05 AM, Georgios Emmanouil <geo.em...@gmail.com> wrote:
> Fixed alignment to match parenthesis.
>
> Signed-off-by: Georgios Emmanouil <geo.em...@gmail.com>

Reviewed-by: Julian Calaby <julian.cal...@gmail.com>

> ---
>  drivers/staging/wilc1000/wilc_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Julian Calaby

Email: julian.cal...@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/


  1   2   3   4   5   >